[PATCH] D83654: [PowerPC] Support constrained vector fp/int conversion

Qiu Chaofan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 17 03:09:03 PDT 2020


qiucf marked 2 inline comments as done.
qiucf added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:8638
 
   return DAG.getNode(Opc, dl, Op.getValueType(), Extend);
 }
----------------
uweigand wrote:
> It would seem that this doesn't create a correct node when called with a STRICT version of Opc -- for one, in this case it will need to handle chains correctly (in and out).
> 
> It is strange that this was not detected by any tests -- is the coverage good enough?
> 
> The rest of the algorithm seems OK for the strict case, since it only introduces integer operations.
Yes, it was missed here. Thanks for comments!

This method is for custom lowering `v4i8/v4i16`. They are not set to `custom` in this patch so such cases will be expanded automatically. After setting them, it hits an assert since PPC doesn't override `TargetLowering::LowerOperationWrapper`. This can be done in later patches.

```
if (SDValue Res = LowerOperation(SDValue(N, 0), DAG))
  Results.push_back(Res);
```

BTW, the method looks strange to me, maybe we can fill rest of operands in base class implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83654





More information about the llvm-commits mailing list