[IR] Add a Instruction::dropPoisonGeneratingFlags helper

[sanjoy]

Summary

The helper will be used in a later change. This change itself is NFC
since the only user of this new function is its unit test.

[majnemer]

lib/IR/Instruction.cpp

126 ↗

(On Diff #89162)

What about the FP operations?

143 ↗

(On Diff #89162)

What about inrange?

[sanjoy]

lib/IR/Instruction.cpp

126 ↗

(On Diff #89162)

None of the floating point operations are specified to generate poison. Are you suggesting that we interpret flags like nsz like we interpret nsw? If so, I think that's viable, but the docs need to change first. Personally I'd say that kind of stuff is blocked on integrating the new poison semantics into LLVM.

143 ↗

(On Diff #89162)

Reading the language ref, inrange does not fit into the poison mould very well.

Say you have

struct S {
  struct {
    int arr[2];
  } t0;
  struct {
    int arr[2];
  } t1;
};

which in IR would be

%struct.S = type { %struct.anon, %struct.anon.0 }
%struct.anon = type { [2 x i32] }
%struct.anon.0 = type { [2 x i32] }

if I'm reading the ref correctly (am I?), then in

%a = getelementptr %struct.S, %struct.S* %0, i64 0, inrange i32 0, i32 0, i64 0
%b = getelementptr i32, i32* %a, i32 2
%c = load i32, i32* %b

the load has undefined behavior. In the poison view of things, we'd like to say that either %a or %b was poison (with the caveat that %b can be generating poison itself, or it could have just passed along the poison it got from %a). Now %a cannot be poison because we want to allow loads and stores through it as usual, which means %b was poison. But that's odd -- %b took a non-poison value as input which it converted into poison by normal wrapping arithmetic (and there is no flag I can strip from that GEP to have it produce a safe (non-poison) value). This means with inrange in the picture, non-inbounds, non-inrange GEPs are no longer "just" additions, multiplications and sign extends.

[majnemer]

Out of curiosity, why not use clearSubclassOptionalData in place of wherever you intend to use dropPoisonGeneratingFlags?

lib/IR/Instruction.cpp

126 ↗

(On Diff #89162)

Considering that we perform reassociation type optimizations based off those fast math flags, I think they are already defacto poison whether they want to be or not ;)

However, we shouldn't gate this change on that.

143 ↗

(On Diff #89162)

Hmm, according to the docs:

Note that the inrange keyword is currently only allowed in constant getelementptr expressions.

We can punt on this I guess.

[sanjoy]

In D30184#681956, @majnemer wrote:

Out of curiosity, why not use clearSubclassOptionalData in place of wherever you intend to use dropPoisonGeneratingFlags?

That seems like a layering problem -- there is no guarantee (is there?) that the bits in SubclassOptionalData correspond to poison. We could conceivably add bits there in the future that (say) denote some probabilistic information (the result of this add is probably positive). That would satisfy the contract of SubclassOptionalData, but would not require to be dropped in dropPoisonGeneratingFlags.

lib/IR/Instruction.cpp

143 ↗

(On Diff #89162)

Okay.

I think the problem above still applies (i.e. right now in LLVM a non-flagged GEP can produce poison), but I do like punting. :)

[efriedma]

lib/IR/Instruction.cpp

126 ↗

(On Diff #89162)

Considering that we perform reassociation type optimizations based off those fast math flags, I think they are already defacto poison whether they want to be or not ;)

A "fast" fadd returns an arbitrary value each time it's executed, but that's not the same thing as poison. (There isn't any optimization reason to introduce undefined behavior into floating-point math, as far as I know.)

[sanjoy]

Ping - @majnemer @efriedma - are there any issues remaining in this patch that I need to address?

[efriedma]

LGTM.

llvm/trunk/include/llvm/IR/Instruction.h

public:
   void setIsExact(bool b = true);
 
   /// Determine whether the no unsigned wrap flag is set.
   bool hasNoUnsignedWrap() const;
 
   /// Determine whether the no signed wrap flag is set.
   bool hasNoSignedWrap() const;
 
+  /// Drops flags that may cause this instruction to evaluate to poison despite
+  /// having non-poison inputs.
+  void dropPoisonGeneratingFlags();
+
   /// Determine whether the exact flag is set.
   bool isExact() const;
 
   /// Set or clear the unsafe-algebra flag on this instruction, which must be an
   /// operator which supports this flag. See LangRef.html for the meaning of
   /// this flag.
   void setHasUnsafeAlgebra(bool B);
 

llvm/trunk/lib/IR/Instruction.cpp

 bool Instruction::hasNoUnsignedWrap() const {
   return cast<OverflowingBinaryOperator>(this)->hasNoUnsignedWrap();
 }
 
 bool Instruction::hasNoSignedWrap() const {
   return cast<OverflowingBinaryOperator>(this)->hasNoSignedWrap();
 }
 
+void Instruction::dropPoisonGeneratingFlags() {
+  switch (getOpcode()) {
+  case Instruction::Add:
+  case Instruction::Sub:
+  case Instruction::Mul:
+  case Instruction::Shl:
+    cast<OverflowingBinaryOperator>(this)->setHasNoUnsignedWrap(false);
+    cast<OverflowingBinaryOperator>(this)->setHasNoSignedWrap(false);
+    break;
+
+  case Instruction::UDiv:
+  case Instruction::SDiv:
+  case Instruction::AShr:
+  case Instruction::LShr:
+    cast<PossiblyExactOperator>(this)->setIsExact(false);
+    break;
+
+  case Instruction::GetElementPtr:
+    cast<GetElementPtrInst>(this)->setIsInBounds(false);
+    break;
+  }
+}
+
 bool Instruction::isExact() const {
   return cast<PossiblyExactOperator>(this)->isExact();
 }
 
 void Instruction::setHasUnsafeAlgebra(bool B) {
   assert(isa<FPMathOperator>(this) && "setting fast-math flag on invalid op");
   cast<FPMathOperator>(this)->setHasUnsafeAlgebra(B);
 }

llvm/trunk/unittests/IR/InstructionsTest.cpp

 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/NoFolder.h"
 #include "llvm/IR/Operator.h"
 #include "gtest/gtest.h"
 #include <memory>
 
 namespace llvm {
 namespace {
 
 TEST(InstructionsTest, ReturnInst) {
TEST(InstructionsTest, AlterInvokeBundles) {
   EXPECT_EQ(Invoke->getArgOperand(0), Clone->getArgOperand(0));
   EXPECT_EQ(Invoke->getCallingConv(), Clone->getCallingConv());
   EXPECT_TRUE(Clone->hasFnAttr(Attribute::AttrKind::Cold));
   EXPECT_EQ(Invoke->getDebugLoc(), Clone->getDebugLoc());
   EXPECT_EQ(Clone->getNumOperandBundles(), 1U);
   EXPECT_TRUE(Clone->getOperandBundle("after").hasValue());
 }
 
+TEST_F(ModuleWithFunctionTest, DropPoisonGeneratingFlags) {
+  auto *OnlyBB = BasicBlock::Create(Ctx, "bb", F);
+  auto *Arg0 = &*F->arg_begin();
+
+  IRBuilder<NoFolder> B(Ctx);
+  B.SetInsertPoint(OnlyBB);
+
+  {
+    auto *UI =
+        cast<Instruction>(B.CreateUDiv(Arg0, Arg0, "", /*isExact*/ true));
+    ASSERT_TRUE(UI->isExact());
+    UI->dropPoisonGeneratingFlags();
+    ASSERT_FALSE(UI->isExact());
+  }
+
+  {
+    auto *ShrI =
+        cast<Instruction>(B.CreateLShr(Arg0, Arg0, "", /*isExact*/ true));
+    ASSERT_TRUE(ShrI->isExact());
+    ShrI->dropPoisonGeneratingFlags();
+    ASSERT_FALSE(ShrI->isExact());
+  }
+
+  {
+    auto *AI = cast<Instruction>(
+        B.CreateAdd(Arg0, Arg0, "", /*HasNUW*/ true, /*HasNSW*/ false));
+    ASSERT_TRUE(AI->hasNoUnsignedWrap());
+    AI->dropPoisonGeneratingFlags();
+    ASSERT_FALSE(AI->hasNoUnsignedWrap());
+    ASSERT_FALSE(AI->hasNoSignedWrap());
+  }
+
+  {
+    auto *SI = cast<Instruction>(
+        B.CreateAdd(Arg0, Arg0, "", /*HasNUW*/ false, /*HasNSW*/ true));
+    ASSERT_TRUE(SI->hasNoSignedWrap());
+    SI->dropPoisonGeneratingFlags();
+    ASSERT_FALSE(SI->hasNoUnsignedWrap());
+    ASSERT_FALSE(SI->hasNoSignedWrap());
+  }
+
+  {
+    auto *ShlI = cast<Instruction>(
+        B.CreateShl(Arg0, Arg0, "", /*HasNUW*/ true, /*HasNSW*/ true));
+    ASSERT_TRUE(ShlI->hasNoSignedWrap());
+    ASSERT_TRUE(ShlI->hasNoUnsignedWrap());
+    ShlI->dropPoisonGeneratingFlags();
+    ASSERT_FALSE(ShlI->hasNoUnsignedWrap());
+    ASSERT_FALSE(ShlI->hasNoSignedWrap());
+  }
+
+  {
+    Value *GEPBase = Constant::getNullValue(B.getInt8PtrTy());
+    auto *GI = cast<GetElementPtrInst>(B.CreateInBoundsGEP(GEPBase, {Arg0}));
+    ASSERT_TRUE(GI->isInBounds());
+    GI->dropPoisonGeneratingFlags();
+    ASSERT_FALSE(GI->isInBounds());
+  }
+}
+
 } // end anonymous namespace
 } // end namespace llvm