[PATCH] D29874: [X86] Generate VZEROUPPER for Skylake-avx512

[Michael Liao via Phabricator via llvm-commits]

hliao added inline comments.


================
Comment at: lib/Target/X86/X86VZeroUpper.cpp:159
 
+
 /// Insert a vzeroupper instruction before I.
----------------
Remove this empty line unless it's definitely required for formatting.


================
Comment at: lib/Target/X86/X86VZeroUpper.cpp:222
+    // explicitly specified.
+    if (IsCall && !callHasRegMask(MI))
       continue;
----------------
aaboud wrote:
> craig.topper wrote:
> > aaboud wrote:
> > > craig.topper wrote:
> > > > Why did this change to not check specific register clobbers?
> > > We can get to this point only if ClobberAllTmmReg is true, in such case ClobberAnyYmmReg will also be true, and we will fall-through exactly as we do with this implementation.
> > > This was a kind of did code.
> > > 
> > > Also, can you explain what is unique with "cannot clobber any" and "cannot clobber all" regarding these lines?
> > This is probably the first time I've really looked at this code so I think explain what is unique without more thought.
> Michael added this code about 3 years ago, he might be able to explain better why he implemented it like this.
for CALL, clobberAllTmmReg checks whether it clobbers ALL tmm regs. Here, clobberAnyTmmReg checks whether that call will uses any YMM registers. they are not equivalent.

however, a previous test case added seems not valid any more due to the calling into builtin library _ftol2 is replaced with native code sequence. not sure whether we will have other code to reproduce PR17631.


https://reviews.llvm.org/D29874