This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Also set dead flags of implicit defs added by BuildMI
ClosedPublic

Authored by Pierre-vh on Aug 9 2023, 8:09 AM.

Details

Summary

BuildMI automatically adds the implicit operands of the
instruction. This meant we couldn''t set the dead flag on
dead implicit defs in that case.

Fix it by introducing an opcode to mark a given implicit
def as dead.

Fixes #64565

Diff Detail

Event Timeline

Pierre-vh created this revision.Aug 9 2023, 8:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 8:09 AM
Pierre-vh requested review of this revision.Aug 9 2023, 8:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2023, 8:09 AM
Pierre-vh edited the summary of this revision. (Show Details)Aug 9 2023, 8:11 AM
Pierre-vh updated this revision to Diff 548631.Aug 9 2023, 8:12 AM

Remove useless new testcase

arsenm added inline comments.Aug 9 2023, 8:37 AM
llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h
989

There should be an implicit_defs() that needs no conditions inside the loop

llvm/test/CodeGen/AArch64/GlobalISel/postlegalizer-lowering-swap-compare-operands.mir
99

Lost comment

575

More lost comments

Pierre-vh updated this revision to Diff 548635.Aug 9 2023, 8:43 AM
Pierre-vh marked 3 inline comments as done.

Address comments

arsenm added inline comments.Aug 9 2023, 10:45 AM
llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h
348

Alternatively could give the operand index directly, it should know

llvm/utils/TableGen/GlobalISelMatchTable.cpp
1973–1974

namespace defined twice identically?

1976

Could just emit the operand index, can do that in a follow up if you want

Pierre-vh updated this revision to Diff 548912.Aug 10 2023, 1:08 AM
Pierre-vh marked 3 inline comments as done.

Use implicit op idx

I don't use the true op idx because it's hard (impossible?) to tell from TableGen, considering we can have complex renderers & such.

arsenm accepted this revision.Aug 10 2023, 2:43 PM
This revision is now accepted and ready to land.Aug 10 2023, 2:43 PM
This revision was landed with ongoing or failed builds.Aug 10 2023, 11:38 PM
This revision was automatically updated to reflect the committed changes.