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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: include/llvm/MC/MCInstrAnalysis.h:88
+                                    const MCInst &Inst,
+                                    BitVector &Writes) const;
+
----------------
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.


================
Comment at: lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:323
+    const MCRegisterClass &GR32RC = MRI.getRegClass(X86::GR32RegClassID);
+    const MCRegisterClass &XMMRC = MRI.getRegClass(X86::VR128RegClassID);
+
----------------
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.


https://reviews.llvm.org/D48225