[PATCH] D63215: Fixing @llvm.memcpy not honoring volatile

[JF Bastien via Phabricator via llvm-commits]

jfb added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5767
   if (Src.isUndef())
     return Chain;
 
----------------
Seems like we don't want to nop stuff if volatile. Can you add a FIXME?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5797
+          !isVol /*AllowOverlap*/, DstPtrInfo.getAddrSpace(),
+          SrcPtrInfo.getAddrSpace(), MF.getFunction().getAttributes()))
     return SDValue();
----------------
Names like this are usually written as `/*IsMemset=*/false`. clang-tidy recognizes and checks that style. Can you update to follow this style here and otherwhere in the patch?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5952
   if (Src.isUndef())
     return Chain;
 
----------------
Ditto on nop'ing volatile.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5976
+          false /*AllowOverlap*/, DstPtrInfo.getAddrSpace(),
+          SrcPtrInfo.getAddrSpace(), MF.getFunction().getAttributes()))
     return SDValue();
----------------
Seems you want `!isVol` for `AllowOverlap` here as well.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6056
   if (Src.isUndef())
     return Chain;
 
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63215