[PATCH] D69891: [VP,Integer,#1] Vector-predicated integer intrinsics

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added inline comments.


================
Comment at: llvm/lib/IR/IntrinsicInst.cpp:276
+  auto VLConst = dyn_cast<ConstantInt>(VLParam);
+  if (VLConst && VLConst->getSExtValue() == -1)
+    return true;
----------------
simoll wrote:
> andrew.w.kaylor wrote:
> > Why are you doing this and not just "less than zero"?
> Because with the new LangRef wording %evl is unsigned and the `i32 -1` constant is handled as a special case.
I see that you also discussed this with Eli. I don't like the fact that constant -1 and a value that is 0xFFFFFFFF at runtime could have different behaviors. There's no telling when the constant folder might find a path to convert a value to a constant. I understand that the intrsincs aren't supposed to be called with values outside the range of [0, W] and has undefined behavior if it is, but having an exception for a constant does seem right.

As a completely contrived example, suppose I had IR like this:

```
bb1:
  br i1 %flag, label %bb2, label %bb3

bb2:
  %t1 = call <8 x i32> @llvm.vp.add.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %n)
  br label %bb4

bb3:
  %t2 = call <8 x i32> @llvm.vp.add.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 -1)
  br label %bb4

bb4:
  %t3 = phi <8 x i32> [%t1, %bb1], [%t2, %bb2]
  ...
```  
Now some pass is going to transform that into:

```
bb1:
  %evl = select i1 %flag, i32 %n, i32 -1
  %t3 = call <8 x i32> @llvm.vp.add.v8i32(<8 x i32> %i0, <8 x i32> %i1, <8 x i1> %m, i32 %evl)
  ...
```  
And now the -1 case has undefined behavior.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69891/new/

https://reviews.llvm.org/D69891