[PATCH] D110311: [IR] Change the default value of InstertElement to poison (1/4)

[Hyeongyu Kim via Phabricator via llvm-commits]

hyeongyukim added a comment.

> Q: Is there any test that is affected by this?

No, tests were not changed by this patch.

This patch is changing the InsertElement's placeholder to poison without changing the LLVM's behavior.
Detailed information on why InsertElement's placeholder should be changed from noundef to poison can be found at https://bugs.llvm.org/show_bug.cgi?id=44185.

Even though this patch is not changing LLVM's behavior, I expect the vector's initialization will be more consistent.



================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:2420
       Type *I32Ty = Type::getInt32Ty(C);
-      Rep = UndefValue::get(VecTy);
       for (unsigned I = 0; I < EltNum; ++I)
----------------
aqjune wrote:
> This change is fine because the `for` loop below fully hides this value, am I correct?
Right, other values are always cover the poisons.
`Rep` is declared as `<Elnum x EltTy> poison,` and for loop is filling `Rep` with a new value from the 0th position to the `EltNum-1`th position. Therefore, all poison in Rep is changed to different values.
Since noundef has not been created before, this patch neither changes the noundef to poison nor makes a new poison.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp:215
   // elements.
-  Value *NewValue = UndefValue::get(C->getType());
   if (isa<ConstantVector>(C)) {
----------------
aqjune wrote:
> Is this poison always hidden behind the newly created `insertelement`s below?
> In other words, is it possible for this patch to cause a transformation like
> <undef, 1, 2> => <poison, mapped(1), mapped(2)>?
Whether C is a `ConstantVector` or a `ConstantAggregate,` the noundef is not changed to poison.
Like the function `UpgradeIntrinsicCall` above, all poisons in NewValue are replaced with `NewOperand` since the for loop is growing 0 to `OperandNum-1`. So all poisons of NewValue will be removed.
Also, since all `NewOperandes` are made through recursive function calls, undef is not changed to poison even inside `NewOperands.`

Even in this case, there is no change in the operation before and after the patch.



================
Comment at: llvm/lib/Target/PowerPC/PPCTargetTransformInfo.cpp:136
             IC.Builder.CreateBitCast(II.getArgOperand(1), Mask->getType());
-        Value *Result = UndefValue::get(Op0->getType());
 
----------------
aqjune wrote:
> I'm not sure about the validity of this change; could you explain the context a bit, please?
I can't figure out the exact context either. 
It is hard to ensure that there are no changes in LLVM before and after this patch, I will revert this part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110311