Page MenuHomePhabricator

[GlobalISel] Change asserting conditions when initializing call site info
Needs ReviewPublic

Authored by matejam on Oct 1 2020, 8:58 AM.

Details

Reviewers
djtodoro
arsenm
Summary

IRTranslator's method runOnMachineFunction is used to translate from IR to MIR when using GlobalISel as an instruction selector.
When all IR instructions are translated, we move all the instructions from the current entry block to it's only successor, which leaves the current entry block empty and useless, we then delete it from the machine function.
The only issue is that the surviving entry block's number is greater by 1 than the minimal entry block number, which is fixed by changing the condition in MIRParserImpl::initializeCallSiteInfo, the idea is to have a special case when using GlobalISel. One way to figure out is GlobalISel the current selector is by not finding "bb.0" in the body of the machine function, the reason for that is beacuse GlobalISel will always start from bb.1, unlike SDAGISel or FastISel.

Diff Detail

Unit TestsFailed

TimeTest
1,100 mslinux > libarcher.parallel::parallel-nosuppression.c
Script: -- : 'RUN: at line 15'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -Wl,-rpath,/mnt/disks/ssd0/agent/llvm-project/build/lib /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/parallel/parallel-nosuppression.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-nosuppression.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0' /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-nosuppression.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/parallel/Output/parallel-nosuppression.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/parallel/parallel-nosuppression.c

Event Timeline

matejam created this revision.Oct 1 2020, 8:58 AM
matejam requested review of this revision.Oct 1 2020, 8:58 AM
arsenm added inline comments.Oct 8 2020, 7:12 AM
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
372–373

I think treating this as some difference to tolerate is the wrong approach. There's no reason the block numbers should change based on the selector

llvm/test/CodeGen/X86/globalisel-entry-block-enumeration.ll
2–3

A pure input MIR test in test/CodeGen/MIR would be preferable

djtodoro added inline comments.Oct 8 2020, 7:54 AM
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
372–373

Hi @arsenm, do you think the entry bb number should start from 0 in the case of GlobalIsel as well? If that is the case, is the D87902 right solution?

matejam added inline comments.Oct 8 2020, 8:13 AM
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
372–373

I agree with @djtodoro , entry bb number in GlobalISel start from 1 by default, unlike in other selectors where they start from 0. In case of a machine function which has only one entry bb it's number will be 1 and the MF.size() will also be 1 which will result in an error "if (BlockNum >= MF.size())".

llvm/test/CodeGen/X86/globalisel-entry-block-enumeration.ll
2–3

It's an .ll test because in the translation process from IR to MIR (IRTranslator in case of using GlobalISel) the generated MIR code won't start from entry bb.0, that is what this test is checking.