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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26255
+
+  bool HasRedZone = Subtarget.isTargetLinux() && Subtarget.isTarget64BitLP64(); 
+  int SPOffset = HasRedZone ? -8 : 0;
----------------
jfb wrote:
> I'd rather see something more precise here.
> 
> We have a `noredzone` function attribute which should be honored:
> https://releases.llvm.org/3.7.0/docs/LangRef.html#id630
> 
> aarch64 has some handling for redzone too:
> https://llvm.org/doxygen/AArch64FrameLowering_8cpp_source.html#l00186
> 
> So does X86FrameLowering.cpp, X86InstrInfo.cpp, and X86MachineFunctionInfo.h.
> 
> Some other architectures to this too. I think you should do the same: if the attribute isn't present, redzone away. If some targets don't have a redzone yet don't set the attribute, they've already opened themselves to pain. Maybe we need a PSA on the mailing list to make sure these targets set `noredzone` on all their functions.
But clang doesn't currently do that does it? Wouldn't it also create a bitcode backwards compatibility issue?


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

https://reviews.llvm.org/D58632