[PATCH] D28744: [X86][AVX] Remove "OptForSize" condition from some memory foldings.

[Ayman Musa via Phabricator via llvm-commits]

aymanmus added inline comments.


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:1928
+    { X86::VRCP14SSrr,        X86::VRCP14SSrm,          TB_NO_REVERSE },
+    { X86::VRSQRT14SSrr,      X86::VRSQRT14SSrm,        TB_NO_REVERSE },
     { X86::VSHUFPDZrri,       X86::VSHUFPDZrmi,         0 },
----------------
RKSimon wrote:
> aymanmus wrote:
> > RKSimon wrote:
> > > Isn't it only the _Int variants that need TB_NO_REVERSE ?
> > TB_NO_REVERSE is used when the load size of the memory form is smaller than the register's size of the register form. In that case the unfolding is not legal since it will produce a load with the registers size.
> I maybe reading the avx512_fp14_s definitions incorrectly but it appears to be using the scalar f32x_info type not the packed equivalents. @craig.topper / @igorb please can you confirm?
That's right, but if you track the definitions more deeply you can see that the "rr" version is defined to have an xmm register operand while the "rm" version is defined to have a 32-bit memory operand.
Unfolding from the memory form to the register form will generate a 128-bit load (because of the xmm register), which we want to avoid.


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:1932
+    { X86::VSQRTSSZr,         X86::VSQRTSSZm,           0 },
+    { X86::VSQRTSDZr,         X86::VSQRTSDZm,           0 },
     { X86::VSUBPDZrr,         X86::VSUBPDZrm,           0 },
----------------
RKSimon wrote:
> aymanmus wrote:
> > RKSimon wrote:
> > > All these changes to X86InstrInfo.cpp should probably be split off - add them to the stack-folding-*.ll tests
> > Do you mean these changes should be in a separate patch?
> > If I understand you correctly, I think the changes in X86InstrInfo.cpp is relevant to the patch since the main change here is to enable the folding of few instructions. Adding these entries does exactly that.
> OK - but please add them to the stack-folding-*.ll tests as well.
Already added (in this patch).


https://reviews.llvm.org/D28744