[PATCH] D101383: [PowerPC] Enable safe for 32bit vins* P10 instructions

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 10 04:20:48 PDT 2021


nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

LGTM other than the nit that can be addressed on the commit.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:2769-2770
+            (VINSWLX $vDi, $rB, Bitcast.FltToInt)>;
+  def : Pat<(v4f32 (PPCvecinsertelt v4f32:$vDi, (f32 (load iaddr:$rA)),
+             i32:$rB)),
+            (VINSWLX $vDi, $rB, (LWZ memri:$rA))>;
----------------
The operand should be lined up with the first operand of the node it belongs to:
```
def : Pat<(v4f32 (PPCvecinsertelt v4f32:$vDi, (f32 (load iaddr:$rA)),
                                  i32:$rB)),
```
Similarly on other similar lines.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:2771
+            (VINSWLX $vDi, $rB, (LWZ memri:$rA))>;
+  def : Pat<(v4f32 (PPCvecinsertelt v4f32:$vDi, (f32 (load iaddrX34:$rA)), i32:$rB)),
+            (VINSWLX $vDi, $rB, (PLWZ memri34:$rA))>;
----------------
ZarkoCA wrote:
> nemanjai wrote:
> > Nit: line too long (here and elsewhere).
> I noticed that but also there are several lines in `IsISA3_1, HasVSX, IsLittleEndian` and `IsISA3_1, HasVSX, IsBigEndian, IsPPC64` that were too long as well.  Thought it may be ok but I fixed it now for this case. 
We haven't done super well with keeping the lines in target description files to 80 lines, but we should still try to do so on new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101383



More information about the llvm-commits mailing list