[PATCH] D30184: [IR] Add a Instruction::dropPoisonGeneratingFlags helper

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/IR/Instruction.cpp:126
+void Instruction::dropPoisonGeneratingFlags() {
+  switch (getOpcode()) {
+  case Instruction::Add:
----------------
majnemer wrote:
> What about the FP operations?
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.


================
Comment at: lib/IR/Instruction.cpp:143
+  case Instruction::GetElementPtr:
+    cast<GetElementPtrInst>(this)->setIsInBounds(false);
+    break;
----------------
majnemer wrote:
> What about `inrange`?
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.



https://reviews.llvm.org/D30184