[PATCH] D78132: [PowerPC] Refactor PPCInstrVSX.td

Stefan Pintilie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 15 04:20:50 PDT 2020


stefanp added a comment.

I like the way that this has been broken down into sections it will make our lives easier for future modifications.



================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:1668
+                          "#DFSTOREf64",
+                          [(store f64:$XT, iaddrX4:$dst)]>;
 
----------------
Do DFLOAD/DFSTORE need mayLoad / mayStore?
I know the original code did not have that so there may be a good reason for it.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2307
+//---------------------------- Anonymous Patterns ----------------------------//
+// Predicate combinations available:
+// [HasVSX]
----------------
jsji wrote:
> How do we intend to maintain this `combinations` list here? What order is this?  Can we maintain a order that is easier to follow?
I do like the list idea since it is easier to see if something already exists.
However, I agree with Jinsong that we need a specific ordering otherwise we will be adding duplicates.
An idea for ordering: 
```
All HasP8Vector
All HasP9Vector
All HasP9Altivec
All IsISAS3_0
Everything Else
```
It does not have to be this ordering but something to make duplicates easy to spot so that we only have to look at a small piece of the list in order to know if a section is there or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78132





More information about the llvm-commits mailing list