[PATCH] D27901: [X86][[AVX512] Code size reduction in X86 by replacing EVEX with VEX encoding

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86EvexToVex.cpp:206
+      
+      if (X86::VR256RegClass.contains(Reg) ||
+          X86::VR128RegClass.contains(Reg))
----------------
gadi.haber wrote:
> craig.topper wrote:
> > delena wrote:
> > > craig.topper wrote:
> > > > gadi.haber wrote:
> > > > > craig.topper wrote:
> > > > > > Can we use TargetRegisterInfo::getRegEncoding() to simplify this?
> > > > > I can simplify the code by defining a lambda function called isAVX512RegOnly(unsigned Reg) and use it to check on the Reg operand to be between X86::ZMM0 and X86::ZMM31 or between X86::YMM16 and X86::YMM31 or between X86:XMM16 and X86::XMM31 as follows:
> > > > > 
> > > > > auto isAVX512RegOnly = [](unsigned Reg) {
> > > > >     if (Reg >= X86::ZMM0 && Reg <= X86::ZMM31)
> > > > >         return true;
> > > > > 
> > > > >     if (Reg >= X86::XMM16 && Reg <= X86::XMM31)
> > > > >         return true;
> > > > > 
> > > > >     if (Reg >= X86::YMM16 && Reg <= X86::YMM31)
> > > > >         return true;
> > > > > 
> > > > >     return false;
> > > > >  };
> > > > > 
> > > > > 
> > > > > 
> > > > Is it even possible to get a VR512 on any of the instructions in the table? The register class is tightly bound to the instruction.
> > > > 
> > > > For XMM/YMM, getRegEncoding would return 0-31 and you can just check that.
> > > I can't find any getRegEncoding() method, do you mean to write a new one?
> > Sorry, it's getEncodingValue().
> True. There shouldn't be any VR512 registers in the EVEX to VEX tables.
> This is an additional safety check.
> 
If it shouldn't happen, shouldn't it just be an assert?


================
Comment at: lib/Target/X86/X86EvexToVex.cpp:196
+    // Check that operands are not ZMM regs or
+    // XMM/YMM regs with hi indexes between 6 - 31.
+    bool IsFoundHiRegVal = false;
----------------
I think this comment should say "16" instead of "6".


Repository:
  rL LLVM

https://reviews.llvm.org/D27901