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

Wed Nov 20 08:53:00 PST 2019

vhscampos marked an inline comment as done.
vhscampos added inline comments.

================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:8969
+      !Subtarget->isThumb1Only() &&
+      LD->getExtensionType() == ISD::NON_EXTLOAD && LD->isVolatile()) {
+    SDLoc dl(N);
----------------
efriedma wrote:
> efriedma wrote:
> > I'd prefer not to exclude extending loads here.  Could lead to weird cases where we miss the transform.
> You still need to *handle* extending loads somehow...
> 
> Actually, maybe we don't mess with volatile loads in DAGCombine, and you don't need to implement it.  In that case, it would still be nice to have an assertion, in case someone changes it at some point.
Your last comment on this wasn't clear to me. If after the latest change you still want me to add anything, please let me know.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70072