From: Roman Lebedev Date: Sun, 20 Jun 2021 09:18:15 +0000 (+0300) Subject: [SimplifyCFG] HoistThenElseCodeToIf(): don't hoist if either block has it's address... X-Git-Tag: llvmorg-14-init~3528 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=ad87761925c2790aab272138b5bbbde4a93e0383;p=platform%2Fupstream%2Fllvm.git [SimplifyCFG] HoistThenElseCodeToIf(): don't hoist if either block has it's address taken This problem is exposed by D104598, after it tail-merges `ret` in `@test_inline_constraint_S_label`, the verifier would start complaining `invalid operand for inline asm constraint 'S'`. Essentially, taking address of a block is mismodelled in IR. It should probably be an explicit instruction, a first one in block, that isn't identical to any other instruction of the same type, so that it can't be hoisted. --- diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 642320e..4f3559d9 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -1404,6 +1404,12 @@ bool SimplifyCFGOpt::HoistThenElseCodeToIf(BranchInst *BI, BasicBlock *BB1 = BI->getSuccessor(0); // The true destination. BasicBlock *BB2 = BI->getSuccessor(1); // The false destination + // If either of the blocks has it's address taken, then we can't do this fold, + // because the code we'd hoist would no longer run when we jump into the block + // by it's address. + if (BB1->hasAddressTaken() || BB2->hasAddressTaken()) + return false; + BasicBlock::iterator BB1_Itr = BB1->begin(); BasicBlock::iterator BB2_Itr = BB2->begin(); diff --git a/llvm/test/CodeGen/AArch64/inlineasm-S-constraint.ll b/llvm/test/CodeGen/AArch64/inlineasm-S-constraint.ll index 3fb2a3f..0e11695 100644 --- a/llvm/test/CodeGen/AArch64/inlineasm-S-constraint.ll +++ b/llvm/test/CodeGen/AArch64/inlineasm-S-constraint.ll @@ -18,3 +18,16 @@ loc: loc2: ret i32 42 } +define i32 @test_inline_constraint_S_label_tailmerged(i1 %in) { +; CHECK-LABEL: test_inline_constraint_S_label_tailmerged: + call void asm sideeffect "adr x0, $0", "S"(i8* blockaddress(@test_inline_constraint_S_label_tailmerged, %loc)) +; CHECK: adr x0, .Ltmp{{[0-9]+}} +br i1 %in, label %loc, label %loc2 +loc: + br label %common.ret +loc2: + br label %common.ret +common.ret: + %common.retval = phi i32 [ 0, %loc ], [ 42, %loc2 ] + ret i32 %common.retval +} diff --git a/llvm/test/Transforms/SimplifyCFG/hoist-from-addresstaken-block.ll b/llvm/test/Transforms/SimplifyCFG/hoist-from-addresstaken-block.ll new file mode 100644 index 0000000..d00593e --- /dev/null +++ b/llvm/test/Transforms/SimplifyCFG/hoist-from-addresstaken-block.ll @@ -0,0 +1,21 @@ +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py +; RUN: opt < %s -mem2reg -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S | FileCheck %s + +define i32 @test_inline_constraint_S_label_tailmerged(i1 %in) { +; CHECK-LABEL: @test_inline_constraint_S_label_tailmerged( +; CHECK-NEXT: call void asm sideeffect "adr x0, $0", "S"(i8* blockaddress(@test_inline_constraint_S_label_tailmerged, [[COMMON_RET:%.*]])) +; CHECK-NEXT: [[COMMON_RETVAL:%.*]] = select i1 [[IN:%.*]], i32 0, i32 42 +; CHECK-NEXT: br label [[COMMON_RET]] +; CHECK: common.ret: +; CHECK-NEXT: ret i32 [[COMMON_RETVAL]] +; + call void asm sideeffect "adr x0, $0", "S"(i8* blockaddress(@test_inline_constraint_S_label_tailmerged, %loc)) +br i1 %in, label %loc, label %loc2 +loc: + br label %common.ret +loc2: + br label %common.ret +common.ret: + %common.retval = phi i32 [ 0, %loc ], [ 42, %loc2 ] + ret i32 %common.retval +}