[PATCH] D58632: [X86] Improve lowering of idemptotent RMW operations

[Philip Reames via Phabricator via llvm-commits]

reames marked 4 inline comments as done.
reames added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:25984
+
+  // NOTE TO REVIEWERS: Do all x86 calling conventions have a redzone?  I
+  // suspect not, but can't find this property to test again.  (usesRedZone on
----------------
craig.topper wrote:
> Windows 64 definitely doesn't have a red zone. I'm not sure if any 32-bit target does.
Does my whitelist approach seem reasonable here?  Or do you have a better suggestion?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:25999
+      Chain};
+    SDNode *Res = DAG.getMachineNode(X86::LOCK_OR64mi32, DL, MVT::Other, Ops);
+    return SDValue(Res, 0);
----------------
craig.topper wrote:
> Shouldn't this be mi8 for a shorter encoding?
> 
> Does 64-bit really need a 64-bit access or can we just use a 32-bit access?

For the 64 bit, we need a valid stack location.  There's no guarantee that ESP will contain a valid address.  (Since it's the 32 bit truncation of RSP and the stack may be outside the low 4 GB.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58632/new/

https://reviews.llvm.org/D58632