[PATCH] D70072: [ARM] Improve codegen of volatile load/store of i64

Mon Nov 11 13:07:36 PST 2019

efriedma added a comment.

The architecture specification provides limited guarantees here, but I guess this doesn't do any harm.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:8969
+      !Subtarget->isThumb1Only() &&
+      LD->getExtensionType() == ISD::NON_EXTLOAD && LD->isVolatile()) {
+    SDLoc dl(N);
----------------
I'd prefer not to exclude extending loads here.  Could lead to weird cases where we miss the transform.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:8983
+    MachineSDNode *Result =
+        DAG.getMachineNode(OpCode, dl, {MVT::i32, MVT::i32, MVT::Other}, Ops);
+    MachineMemOperand *MemOp = cast<MemSDNode>(N)->getMemOperand();
----------------
Please don't make MachineSDNodes this early; it might appear to mostly work, but other code is not expecting MachineSDNodes at this point.  See ARMISelLowering.h for how to introduce a target-specific SDNode.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70072