InstCombine: Can't fold a phi arg load into the phi if the load is from a swifterror address

[aschwaighofer]

Summary

swifterror addresses are only allowed as operands to load, store, and
calls.

The following transformation is not allowed. It would create a phi with a
swifterror address operand.

%addr = alloca swifterror i8*
br %cond, label %bb1, label %b22

bb1:
  %val1 = load i8*, i8** %addr
  br exit

bb2:
  %val2 = load i8*, i8** %addr
  br exit

exit:
  %val = phi [%val1, %bb1] [%val2, %bb2]

>

%addr = alloca swifterror i8*
br %cond, label %bb1, label %b22

bb1:
  br exit

bb2:
  br exit

exit:
  %val_addr = phi [%addr, %bb1] [%addr, %bb2]
  %val2 = load i8*, i8** %val_addr

rdar://89865485

[fhahn]

llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll

581

Looks like this test uses utils/update_test_checks.py to generate the checks. Could you update the test using the script?

[aschwaighofer]

Update test checks to use llvm/utils/update_test_checks.py

[fhahn]

LGTM, thanks! This matches the spec in LangRef.

But in general, I'd expect this restriction not being enforced in many other places. Is there a verifier that rejects invalid uses of swifterror?

llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll

620

nit: remove empty lines

[aschwaighofer]

Yes the LLVM verifier catches invalid uses of swifterror addresses.

[aschwaighofer]

Remove empty lines

llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp

if (AllocaInst *AI = dyn_cast<AllocaInst>(GEP->getOperand(0)))
         return false;
 
   return true;
 }
 
 Instruction *InstCombinerImpl::foldPHIArgLoadIntoPHI(PHINode &PN) {
   LoadInst *FirstLI = cast<LoadInst>(PN.getIncomingValue(0));
 
+  // Can't forward swifterror through a phi.
+  if (FirstLI->getOperand(0)->isSwiftError())
+    return nullptr;
+
   // FIXME: This is overconservative; this transform is allowed in some cases
   // for atomic operations.
   if (FirstLI->isAtomic())
     return nullptr;
 
   // When processing loads, we need to propagate two bits of information to the
   // sunk load: whether it is volatile, and what its alignment is.
   bool IsVolatile = FirstLI->isVolatile();
for (auto Incoming : drop_begin(zip(PN.blocks(), PN.incoming_values()))) {
     if (!LI || !LI->hasOneUser() || LI->isAtomic())
       return nullptr;
 
     // Make sure all arguments are the same type of operation.
     if (LI->isVolatile() != IsVolatile ||
         LI->getPointerAddressSpace() != LoadAddrSpace)
       return nullptr;
 
+    // Can't forward swifterror through a phi.
+    if (LI->getOperand(0)->isSwiftError())
+      return nullptr;
+
     // We can't sink the load if the loaded value could be modified between
     // the load and the PHI.
     if (LI->getParent() != InBB || !isSafeAndProfitableToSinkLoad(LI))
       return nullptr;
 
     LoadAlignment = std::min(LoadAlignment, LI->getAlign());
 
     // If the PHI is of volatile loads and the load block has multiple

llvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll

 exit:
   %ptr.typed = phi i8* [ %ptr1, %bb1 ], [ %ptr2, %bb2 ]
   %res.phi = phi i8 [ %res1, %bb1 ], [ %res2, %bb2 ]
   store i8 1, i8* %ptr.typed
   %res.load = load i8, i8* %ptr.typed
   %res = select i1 %cond2, i8 %res.phi, i8 %res.load
   ret i8 %res
 }
+
+; `swifterror` addresses are restricted to load and stores and call arguments.
+declare void @takeAddress(i8** swifterror)
+
+define i8* @test_dont_optimize_swifterror(i1 %cond, i1 %cond2, i8* %ptr) {
+; INSTCOMBINE-LABEL: @test_dont_optimize_swifterror(
+; INSTCOMBINE-NEXT:  entry:
+; INSTCOMBINE-NEXT:    [[OBJ:%.*]] = alloca swifterror i8*, align 8
+; INSTCOMBINE-NEXT:    [[OBJ2:%.*]] = alloca swifterror i8*, align 8
+; INSTCOMBINE-NEXT:    call void @takeAddress(i8** nonnull swifterror [[OBJ]])
+; INSTCOMBINE-NEXT:    call void @takeAddress(i8** nonnull swifterror [[OBJ2]])
+; INSTCOMBINE-NEXT:    store i8* [[PTR:%.*]], i8** [[OBJ]], align 8
+; INSTCOMBINE-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
+; INSTCOMBINE:       bb1:
+; INSTCOMBINE-NEXT:    [[RES1:%.*]] = load i8*, i8** [[OBJ]], align 8
+; INSTCOMBINE-NEXT:    br label [[EXIT:%.*]]
+; INSTCOMBINE:       bb2:
+; INSTCOMBINE-NEXT:    [[RES2:%.*]] = load i8*, i8** [[OBJ2]], align 8
+; INSTCOMBINE-NEXT:    br label [[EXIT]]
+; INSTCOMBINE:       exit:
+; INSTCOMBINE-NEXT:    [[RES_PHI:%.*]] = phi i8* [ [[RES1]], [[BB1]] ], [ [[RES2]], [[BB2]] ]
+; INSTCOMBINE-NEXT:    store i8* null, i8** [[OBJ]], align 8
+; INSTCOMBINE-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i8* [[RES_PHI]], i8* null
+; INSTCOMBINE-NEXT:    ret i8* [[RES]]
+;
+; INSTCOMBINEGVN-LABEL: @test_dont_optimize_swifterror(
+; INSTCOMBINEGVN-NEXT:  entry:
+; INSTCOMBINEGVN-NEXT:    [[OBJ:%.*]] = alloca swifterror i8*, align 8
+; INSTCOMBINEGVN-NEXT:    [[OBJ2:%.*]] = alloca swifterror i8*, align 8
+; INSTCOMBINEGVN-NEXT:    call void @takeAddress(i8** nonnull swifterror [[OBJ]])
+; INSTCOMBINEGVN-NEXT:    call void @takeAddress(i8** nonnull swifterror [[OBJ2]])
+; INSTCOMBINEGVN-NEXT:    store i8* [[PTR:%.*]], i8** [[OBJ]], align 8
+; INSTCOMBINEGVN-NEXT:    br i1 [[COND:%.*]], label [[BB1:%.*]], label [[BB2:%.*]]
+; INSTCOMBINEGVN:       bb1:
+; INSTCOMBINEGVN-NEXT:    br label [[EXIT:%.*]]
+; INSTCOMBINEGVN:       bb2:
+; INSTCOMBINEGVN-NEXT:    [[RES2:%.*]] = load i8*, i8** [[OBJ2]], align 8
+; INSTCOMBINEGVN-NEXT:    br label [[EXIT]]
+; INSTCOMBINEGVN:       exit:
+; INSTCOMBINEGVN-NEXT:    [[RES_PHI:%.*]] = phi i8* [ [[PTR]], [[BB1]] ], [ [[RES2]], [[BB2]] ]
+; INSTCOMBINEGVN-NEXT:    store i8* null, i8** [[OBJ]], align 8
+; INSTCOMBINEGVN-NEXT:    [[RES:%.*]] = select i1 [[COND2:%.*]], i8* [[RES_PHI]], i8* null
+; INSTCOMBINEGVN-NEXT:    ret i8* [[RES]]
+;
+entry:
+  %obj = alloca swifterror i8*, align 8
+  %obj2 = alloca swifterror i8*, align 8
+  call void @takeAddress(i8** swifterror %obj)
+  call void @takeAddress(i8** swifterror %obj2)
+  store i8* %ptr, i8** %obj, align 8
+  br i1 %cond, label %bb1, label %bb2
+
+bb1:                                              ; preds = %entry
+  %res1 = load i8*, i8** %obj, align 8
+  br label %exit
+
+bb2:                                              ; preds = %entry
+  %res2 = load i8*, i8** %obj2, align 8
+  br label %exit
+
+exit:                                             ; preds = %bb2, %bb1
+  %res.phi = phi i8* [ %res1, %bb1 ], [ %res2, %bb2 ]
+  store i8* null, i8** %obj, align 8
+  %res = select i1 %cond2, i8* %res.phi, i8* null
+  ret i8* %res
+}