This is an archive of the discontinued LLVM Phabricator instance.

Bug fix "GC relocate is incorrectly tied to the statepoint"
ClosedPublic

Authored by dbakunevich on Jun 30 2022, 4:50 AM.

Details

Summary

Make Verifier recognize undef tokens as correct IR

Undef tokens may appear in unreached code as result of RAUW of some optimization,
and it should not be considered as bad IR.

Link to bug: https://github.com/llvm/llvm-project/issues/56267

Diff Detail

Event Timeline

dbakunevich created this revision.Jun 30 2022, 4:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 4:50 AM
dbakunevich requested review of this revision.Jun 30 2022, 4:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 4:50 AM
mkazantsev added inline comments.Jun 30 2022, 4:58 AM
llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
1202

This needs a test.

llvm/lib/IR/IntrinsicInst.cpp
701

remove

dbakunevich added a reviewer: apilipenko.
apilipenko added inline comments.Jul 1 2022, 8:12 PM
llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
173

Assert isa<GCStatepointInst, UndefValue>(Statepoint)

1206

Assert isa<GCStatepointInst, UndefValue>(SI)

1235

Assert isa<GCStatepointInst, UndefValue>(SI)

llvm/lib/IR/Verifier.cpp
5159–5160

isa<GCStatepointInst, UndefValue>(Token)

dbakunevich marked 5 inline comments as done.

As part of this task, I could not use the

isa<Type_1,Type_2>(Val)

construct, because there was an error when building llvm.
Throughout llvm, the construction

isa<Type1>(Val) || isa<Type_2>(Val)

is often used. Therefore, I propose to solve the full transition problem with

isa<>() || isa<>()

to

isa<,>()

as a separate NFC patch

mkazantsev added inline comments.Jul 4 2022, 5:10 AM
llvm/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
173

Not sure if this assert is needed at all, because it will naturally be asserted by cast below.

llvm/lib/IR/IntrinsicInst.cpp
719–724

auto*

721

Remove const_cast

llvm/test/Transforms/InstCombine/gc.relocate-verify.ll
12

unreached

Pls change patch description into smth more verbose, like

Make Verifier recognize undef tokens as correct IR

Undef tokens may appear in unreached code as result of RAUW of some optimization,
and it should not be considered as bad IR.
dbakunevich edited the summary of this revision. (Show Details)
mkazantsev accepted this revision.Jul 5 2022, 1:57 AM

LGTM w/ description fix.

This revision is now accepted and ready to land.Jul 5 2022, 1:57 AM
This revision was landed with ongoing or failed builds.Jul 18 2022, 2:26 AM
This revision was automatically updated to reflect the committed changes.