This is an archive of the discontinued LLVM Phabricator instance.

llvm-reduce: Fix assertion on blockaddress during function reduction
ClosedPublic

Authored by arsenm on Jan 3 2023, 10:13 AM.

Details

Summary

Just avoid crashing for now, we should be able to replace the blockaddresses
themselves.

BlockAddress::handleOperandChangeImpl assumes it can cast to Function.
The verifier seems nonexistent and the langref isn't particularly explicit
on what's allowed as a blockaddress operand. As far as I can tell bugpoint
isn't doing anything to handle this.

Something low level is broken with BlockAddress handling,
demonstrated by reduce-functions-blockaddress-wrong-function.ll.
The BasicBlock destructor of the deleted function is triggering replacement
of blockaddresses for the kept function in some cases. I've only half debugged
this but it seems like blockaddress is handled too-specially compared to other
Constants. I have tentative patches to allow any constant to be a blockaddress
input, but having the verifier check if it's really a function/block.

Diff Detail

Event Timeline

arsenm created this revision.Jan 3 2023, 10:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 10:13 AM
arsenm requested review of this revision.Jan 3 2023, 10:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2023, 10:13 AM
Herald added a subscriber: wdng. · View Herald Transcript
arsenm updated this revision to Diff 486022.Jan 3 2023, 10:30 AM

Use separate file for addrspacecasted case. Something horrible seems to be happening if there are actual uses of the addrspacecasted version in the same file

arsenm updated this revision to Diff 486092.Jan 3 2023, 2:32 PM

Re-merge test. The weird behavior shows up with any other unused function present, I think because the removal logic special cases unused functions

arsenm updated this revision to Diff 486279.Jan 4 2023, 7:09 AM
arsenm edited the summary of this revision. (Show Details)

Add a testcase showing wrong blockaddress replacement if a different function is deleted. I don't think this is a proper llvm-reduce bug and something seems broadly wrong with blockaddress handling

aeubanks accepted this revision.Jan 10 2023, 4:06 PM
aeubanks added inline comments.
llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll
6–7

very weird, no idea why this would be happening. perhaps some cloning issue?

llvm/tools/llvm-reduce/deltas/ReduceFunctions.cpp
36

not sure I agree with this TODO

we should let the alias pass handle aliases and the instruction/globals pass handle blockaddresses, and keep this one simple

This revision is now accepted and ready to land.Jan 10 2023, 4:06 PM