[PATCH] D23370: [CodeGen] Rename MachineInstr::isInvariantLoad to isDereferenceableInvariantLoad. NFC

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 23 12:20:41 PDT 2016


reames added a subscriber: reames.

================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1127
@@ -1126,7 +1126,3 @@
 
-  /// Return true if this instruction is loading from a
-  /// location whose value is invariant across the function.  For example,
-  /// loading a value from the constant pool or from the argument area of
-  /// a function if it does not change.  This should only return true of *all*
-  /// loads the instruction does are invariant (if it does multiple loads).
-  bool isInvariantLoad(AliasAnalysis *AA) const;
+  /// Return true if this load instruction never traps and always returns the
+  /// same value.
----------------
I think this comment is missing something major.  In general, dereferenceability is a context-sensative property.  In this query, the dereferenceable property is not context sensative and can only return true if the location is dereferenceable for any possible context.  That should be clarified.  

The "any possible context" bit is the issue Hal raises about function scope vs global scope.  I believe we want function scope, but that implies we have to strip the relevant flag if we ever inline at the MI level.  Do we actually do that?


https://reviews.llvm.org/D23370





More information about the llvm-commits mailing list