[PATCH] D38738: X86CallFrameOptimization: Recognize 'store 0/-1 using and/or' idioms

[Zvi Rackover via Phabricator via llvm-commits]

zvi added inline comments.


================
Comment at: lib/Target/X86/X86CallFrameOptimization.cpp:283
 
   // The instructions we actually care about are movs onto the stack
+  switch (MI->getOpcode()) {
----------------
aymanmus wrote:
> The comment implies only movs are acceptable.
> Please modify.
ok


================
Comment at: lib/Target/X86/X86CallFrameOptimization.cpp:506
   for (int Idx = (Context.ExpectedDist >> Log2SlotSize) - 1; Idx >= 0; --Idx) {
     MachineBasicBlock::iterator MOV = *Context.MovVector[Idx];
     MachineOperand PushOp = MOV->getOperand(X86::AddrNumOperands);
----------------
aymanmus wrote:
> Here also, the comments and variable names may be misleading.
Will fix the comment at commit-time. The variable names will be changed in a follow-up commit.


================
Comment at: test/CodeGen/X86/movtopush.ll:1
 ; RUN: llc < %s -mtriple=i686-windows | FileCheck %s -check-prefix=NORMAL
 ; RUN: llc < %s -mtriple=i686-windows -no-x86-call-frame-opt | FileCheck %s -check-prefix=NOPUSH
----------------
DavidKreitzer wrote:
> It seems like similar changes are needed in movtopush64.ll.
Will add the required tests.


================
Comment at: test/CodeGen/X86/movtopush.ll:395
 
-; Check that a stack restore (leal -4(%ebp), %esp) doesn't get merged with a
+; hat a stack restore (leal -4(%ebp), %esp) doesn't get merged with a
 ; stack adjustment (addl $12, %esp). Just because it's a lea doesn't mean it's
----------------
aymanmus wrote:
> Deleted word by mistake I guess.
Whoops


https://reviews.llvm.org/D38738