[PATCH] D157273: [GlobalISel] Add dead flags to implicit defs in ISel

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 7 09:38:00 PDT 2023


arsenm added a comment.

In D157273#4565254 <https://reviews.llvm.org/D157273#4565254>, @Pierre-vh wrote:

> In D157273#4565239 <https://reviews.llvm.org/D157273#4565239>, @foad wrote:
>
>> Are there any cases where this improves codegen?
>
> The only codegen change I spotted was in mul-scalar.ll (X86)
> The intent is just to improve correctness. @arsenm noticed that not setting dead flags was an issue in https://github.com/llvm/llvm-project/issues/63986

It's not really a correctness fix, it's just avoiding regressing > 30 globalisel tests when my fix for that is applied. Most of the cases we're compensating for missing constant folding in GlobalISel



================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h:335
   /// - RegNum - The register to add
+  /// - Dead - Whether this is a dead def.
   GIR_AddImplicitDef,
----------------
Can just make the flag value?


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h:390-391
   /// instructions.
-  /// This is less constrained than a custom renderer and can update instructions
+  /// This is less constrained than a custom renderer and can update
+  /// instructions
   /// in the state.
----------------
Unrelated formatting change


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h:951-952
+      unsigned Flags = RegState::Implicit;
+      if (Dead)
+        Flags |= RegState::Dead;
+      OutMIs[InsnID].addDef(RegNum, Flags);
----------------
I'd expect the table entry to just be any flags an unconditionally or'd


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157273



More information about the llvm-commits mailing list