[GuardWidening] Remove nuw/nsw flags for hoisted instructions

[skatkov]

Summary

When we hoist instructions over guard we must clear flags due to these flags
might be implied using this guard, so they make sense only after the guard.

As an example of the bug due to current behavior.
L is known to be in range say [0, 100)
c1 = x u< L
guard (c1)
x1 = add x, 1
c2 = x1 u< L
guard(c2)

basing on guard(c1) we can say that x1 = add nuw nsw x, 1
after guard widening we get
c1 = x u< L
x1 = add nuw nsw x, 1
c2 = x1 u< L
c = and c1, c2
guard(c)

now, basing on fact that x + 1 < L and x >= 0 due to x + 1 is nuw
we can prove that x + 1 u< L implies that x u< L, so we can just remove c1
x1 = add nuw nsw x, 1
c2 = x1 u< L
guard(c2)

But that is not correct due to we will pass x == -1 value.

[mkazantsev]

Looks good, but please regenerate checks here. It's not clear at all what this test is trying to ensure (e.g. what is wide.chk).

llvm/test/Transforms/GuardWidening/range-check-merging.ll

342

Can you please regenerate all checks in this file with update_test_checks.py and then rebase on top of it? I can't say what it's actually trying to check (and same applies to other tests here).

[nikic]

Not familiar with this pass, but I suspect the right fix for this might be to use CreateLogicalAnd instead of CreateAnd.

[skatkov]

In D126354#3536967, @nikic wrote:

Not familiar with this pass, but I suspect the right fix for this might be to use CreateLogicalAnd instead of CreateAnd.

Could you please elaborate on this. I do not follow what you mean.

[skatkov]

In D126354#3536815, @mkazantsev wrote:

Looks good, but please regenerate checks here. It's not clear at all what this test is trying to ensure (e.g. what is wide.chk).

Done.

llvm/lib/Transforms/Scalar/GuardWidening.cpp

void GuardWideningImpl::makeAvailableAt(Value *V, Instruction *Loc) const {
 
   assert(isSafeToSpeculativelyExecute(Inst, Loc, &DT) &&
          !Inst->mayReadFromMemory() && "Should've checked with isAvailableAt!");
 
   for (Value *Op : Inst->operands())
     makeAvailableAt(Op, Loc);
 
   Inst->moveBefore(Loc);
+  // If we moved instruction before guard we must clean nuw, nsw flags.
+  Inst->setHasNoUnsignedWrap(false);
+  Inst->setHasNoSignedWrap(false);
 }
 
 bool GuardWideningImpl::widenCondCommon(Value *Cond0, Value *Cond1,
                                         Instruction *InsertPt, Value *&Result,
                                         bool InvertCondition) {
   using namespace llvm::PatternMatch;
 
   {

llvm/test/Transforms/GuardWidening/range-check-merging.ll

entry:
   ret void
 }
 
 define void @f_8(i32 %x, i32* %length_buf) {
 ; Check that we clean nuw nsw flags
 ; CHECK-LABEL: @f_8(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[LENGTH:%.*]] = load i32, i32* [[LENGTH_BUF:%.*]], align 4, !range [[RNG0]]
 ; CHECK-NEXT:    [[CHK0:%.*]] = icmp ult i32 [[X:%.*]], [[LENGTH]]
-; CHECK-NEXT:    [[X_INC1:%.*]] = add nuw nsw i32 [[X]], 1
+; CHECK-NEXT:    [[X_INC1:%.*]] = add i32 [[X]], 1
 ; CHECK-NEXT:    [[CHK1:%.*]] = icmp ult i32 [[X_INC1]], [[LENGTH]]
 ; CHECK-NEXT:    [[WIDE_CHK:%.*]] = and i1 [[CHK0]], [[CHK1]]
 ; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 [[WIDE_CHK]]) [ "deopt"() ]
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %length = load i32, i32* %length_buf, !range !0
   %chk0 = icmp ult i32 %x, %length