[PATCH] D92789: [PPC] Check for PPC64 when emitting 64bit specific VSX nodes when pattern matching built vectors

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCInstrVSX.td:2412
 // [HasVSX]
 // [HasVSX, IsBigEndian]
 // [HasVSX, IsLittleEndian]
----------------
ZarkoCA wrote:
> Xiangling_L wrote:
> > I am wondering why do we leave alone `[HasVSX, IsBigEndian]` and `[HasVSX, HasOnlySwappingMemOps, IsBigEndian]` and `HasVSX, HasP8Vector, NoP9Vector, IsBigEndian]`?
> That's a good question, I tried disabling these so as to have minimal disruption to existing code. And did it by elimination.  
> 
> So, as far as I can see the potentially big problem in the pattern matching is later implementations like `[HasVSX, IsISA3_0, HasDirectMove, IsBigEndian]` were implemented without considering 32bit codegen and I added a check there and moved on to earlier ones. At some point, I started hitting test case failures and found that codegen in those cases was correct. And in those cases I found that there were checks added for 32bit like in this patch: https://reviews.llvm.org/D17711.  
> 
> In short, I tried to do the safe thing for 32bit VSX without major disruption to existing test cases and codegen unless I could plainly tell it wasn't correct. Which in those cases I wasn't able to. 
I see your point here. I am asking because I tried adding `IsPPC64` for `[HasVSX, HasP8Vector, NoP9Vector, IsBigEndian]` and found no regressions. So I feel the current situation is kinda messy by having partial fixes and disablement for 32bit mode. But I understand that you want this patch to be conservative correct.

I am wondering Is that possible for us to add `IsPPC64` for all predicates where no existing testcases would be failed? But if it would take you too much time, I can live with that for now. Since as we discussed offline that we do have plan to verify them one by one for 32bit mode in the near future.


================
Comment at: llvm/test/CodeGen/PowerPC/ppc-32bit-build-vector.ll:64
+}
+attributes #0 = { "target-features"="+altivec,+bpermd,+crypto,+direct-move,+extdiv,+htm,+power8-vector,+power9-vector,+vsx,-spe" }
----------------
Sorry that I forgot to mention, attributes `attributes #0` are also not necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92789