[PATCH] D147363: [AMDGPU] Add target hook to isGlobalMemoryObject

[Austin Kerbow via Phabricator via llvm-commits]

kerbowa added inline comments.


================
Comment at: llvm/lib/CodeGen/ScheduleDAGInstrs.cpp:537
 
-/// Returns true if MI is an instruction we are unable to reason about
-/// (like a call or something with unmodeled side effects).
-static inline bool isGlobalMemoryObject(MachineInstr *MI) {
+bool ScheduleDAGInstrs::isGlobalMemoryObject(MachineInstr *MI) {
   return MI->isCall() || MI->hasUnmodeledSideEffects() ||
----------------
arsenm wrote:
> I don't understand why you need to special case these. Isn't this more or less the same as removing hasSideEffects from the instruction? Could you achieve the same thing by using a pseudo memory operand that doesn't alias anything the instruction reads and writes from?
Should the intrinsic have sideeffects in that case? We would need some special handling in isel if there is a mismatch between hasSideEffects between the intrinsics and the MachineInstrs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147363