[PATCH] D156520: [TII] NFCI: Simplify the interface for isTriviallyReMaterializable

Piotr Sobczak via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 28 06:52:23 PDT 2023


piotr added a comment.

Thanks, AMD changes LGTM. I spotted this weirdness recently when staring at some remat code for AMDGPU. (It's not a serious issue for us, because the generic check is really conservative for our target, and we typically do not want to block the remat when the generic check returns true).



================
Comment at: llvm/include/llvm/MC/MCInstrDesc.h:522
   ///
-  /// If this flag is set, the isReallyTriviallyReMaterializable()
-  /// or isReallyTriviallyReMaterializableGeneric methods are called to verify
-  /// the instruction is really rematable.
+  /// If this flag is set, the isReallyTriviallyReMaterializable() methods is
+  /// called to verify the instruction is really rematerializable.
----------------
"method is" or "methods are"


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:877
+        const MachineRegisterInfo &MRI = MF.getRegInfo();
+        return regIsPICBase(BaseReg, MRI);
+      }
----------------
Doesn't it need the conditional change as in line 898 below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156520



More information about the llvm-commits mailing list