[PATCH] D32684: [X86] Adding new LLVM TableGen backend that generates the X86 backend memory folding tables.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: test/CodeGen/X86/stack-folding-fp-avx1.ll:1654
 
-define double @stack_fold_sqrtsd(double %a0) {
-  ;CHECK-LABEL: stack_fold_sqrtsd
----------------
aymanmus wrote:
> RKSimon wrote:
> > aymanmus wrote:
> > > RKSimon wrote:
> > > > Please don't delete tests, especially if they've regressed - show the new codegen and add a todo comment
> > > The test checks that specific instructions are memory folded.
> > > In the removed test cases, the instructions were removed from the memory folding tables so testing that these instructions are NOT folded in this test, is not the best idea.
> > OK - so just leave TODO comments for the 4 cases:
> > ```
> > ; TODO stack_fold_sqrtsd
> > ; TODO stack_fold_sqrtsd_int
> > ; TODO stack_fold_sqrtss
> > ; TODO stack_fold_sqrtss_int
> > ```
> > 
> > 
> > 
> Sorry if i'm being too picky about this, but I cannot understand why to add TODOs for these test cases. what should be done with them later on? what would it indicate?
The SSE scalar math memory folding has always been tricky - I had wondered if these cases could be handled by X86InstrInfo::foldMemoryOperandCustom? The _Int variants will probably have to be treated quite differently but it'd be great to get pass-through of the upper bits from the other operand being properly supported some day.

I notice that you didn't have to alter the SSE versions, nor the rsqrt/rcp equivalents, so in the medium term I think we can get this fixed, which is why I say we keep the TODOs there to help keep track. You can strip the actual test cases, just leave the 4 TODO comments.



https://reviews.llvm.org/D32684