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

[Justin Lebar via llvm-commits]

jlebar added inline comments.

================
Comment at: llvm/lib/CodeGen/MachineInstr.cpp:1596
@@ -1595,7 +1595,3 @@
 
-/// isInvariantLoad - 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 MachineInstr::isInvariantLoad(AliasAnalysis *AA) const {
+/// isDereferenceableInvariantLoad - Return true if this instruction will never
+/// trap and is loading from a location whose value is invariant across the
----------------
chandlerc wrote:
> No need to start with the function name.
> 
> Also, I'd format the comment to make it clear what the brief summary is while you're changing it.
> No need to start with the function name.

I don't like it either, but this is the prevailing style in the file, and the LLVM coding standards say (in bold) "If you are extending, enhancing, or bug fixing already implemented code, use the style that is already being used."?

Frankly if I were cleaning it up as a clean slate I'd delete the comment entirely, because it just repeats (verbatim) what's in the header.

Happy to do whatever you want here.

> Also, I'd format the comment to make it clear what the brief summary is while you're changing it.

Done.


https://reviews.llvm.org/D23370