[PATCH] D48225: [llvm-mca][X86] Teach how to identify register writes that implicitly clear the upper portion of a super-register.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

In https://reviews.llvm.org/D48225#1133891, @craig.topper wrote:

> Does this cover writes to YMM clearing the upper half of ZMM registers?


Good point. I didn't consider that case.
I guess we need to conservatively also check for YMM writes. Obviously, the implicit zeroing of the upper half of ZMM would only make sense for targets where ZMM is effectively usable...



================
Comment at: include/llvm/MC/MCInstrAnalysis.h:88
+                                    const MCInst &Inst,
+                                    BitVector &Writes) const;
+
----------------
craig.topper wrote:
> andreadb wrote:
> > RKSimon wrote:
> > > When is it better to use BitVector vs APInt? I don't have an answer but we're incredibly inconsistent on this!
> > I think using a BitVector (at least in this context) is probably okay. But - to be honest - I don't know the right answer to that question either.
> > The idea is to use a simple bitvector to do very simple bit manipulation.
> > 
> > APInt has a much richer interface. It allows to do other things (other than bit manipulation). It allows to do arithmetic and logic computation on integers with arbitrary precision. APInt is probably "over designed" for this particular context.
> > 
> > That being said, both interfaces are okay. If you prefer, I can switch to APInt.
> BitVector always heap allocates, APInt heap allocates above 64 bits. SmallBitVector heap allocates above 58 bits on 64-bit hosts and above 27 bits on 32-bit hosts.
Thanks for the answer. I change it to APInt then.


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:323
+    const MCRegisterClass &GR32RC = MRI.getRegClass(X86::GR32RegClassID);
+    const MCRegisterClass &XMMRC = MRI.getRegClass(X86::VR128RegClassID);
+
----------------
craig.topper wrote:
> andreadb wrote:
> > RKSimon wrote:
> > > Is this safe for i686 32-bit targets?
> > It should be safe for i686 because the super-register of a 32-bit GPR is not usable/existent in practice.
> VR128RegClass doesn't include XMM16-XMM31. Those are in VR128XRegClass.
I will change it. Thanks.


https://reviews.llvm.org/D48225