[GlobalISel] Change asserting conditions when initializing call site info

[matejam]

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.

[arsenm]

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]

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]

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.

[arsenm]

llvm/lib/CodeGen/MIRParser/MIRParser.cpp

372–373

It's the same MIR. The numbering doesn't change. There's something pointlessly inconsistent if the block numbering ends up different

[arsenm]

I think the block numbering is meaningless and shouldn't be any different between selectors

llvm/lib/CodeGen/MIRParser/MIRParser.cpp

 
 bool MIRParserImpl::initializeCallSiteInfo(
     PerFunctionMIParsingState &PFS, const yaml::MachineFunction &YamlMF) {
   MachineFunction &MF = PFS.MF;
   SMDiagnostic Error;
   const LLVMTargetMachine &TM = MF.getTarget();
   for (auto YamlCSInfo : YamlMF.CallSitesInfo) {
     yaml::CallSiteInfo::MachineInstrLoc MILoc = YamlCSInfo.CallLocation;
-    if (MILoc.BlockNum >= MF.size())
+    unsigned BlockNum = MILoc.BlockNum;
+    // In case of using GlobalISel, entry block enumeration doesn't start
+    // from 0, but from 1.
+    if (YamlMF.Body.Value.Value.find("bb.0") == std::string::npos)
+      BlockNum -= 1;
+    if (BlockNum >= MF.size())
       return error(Twine(MF.getName()) +
                    Twine(" call instruction block out of range.") +
-                   " Unable to reference bb:" + Twine(MILoc.BlockNum));
-    auto CallB = std::next(MF.begin(), MILoc.BlockNum);
+                   " Unable to reference bb:" + Twine(BlockNum));
+    auto CallB = std::next(MF.begin(), BlockNum);
     if (MILoc.Offset >= CallB->size())
       return error(Twine(MF.getName()) +
                    Twine(" call instruction offset out of range.") +
                    " Unable to reference instruction at bb: " +
-                   Twine(MILoc.BlockNum) + " at offset:" + Twine(MILoc.Offset));
+                   Twine(BlockNum) + " at offset:" + Twine(MILoc.Offset));
     auto CallI = std::next(CallB->instr_begin(), MILoc.Offset);
     if (!CallI->isCall(MachineInstr::IgnoreBundle))
       return error(Twine(MF.getName()) +
                    Twine(" call site info should reference call "
                          "instruction. Instruction at bb:") +
-                   Twine(MILoc.BlockNum) + " at offset:" + Twine(MILoc.Offset) +
+                   Twine(BlockNum) + " at offset:" + Twine(MILoc.Offset) +
                    " is not a call instruction");
     MachineFunction::CallSiteInfo CSInfo;
     for (auto ArgRegPair : YamlCSInfo.ArgForwardingRegs) {
       Register Reg;
       if (parseNamedRegisterReference(PFS, Reg, ArgRegPair.Reg.Value, Error))
         return error(Error, ArgRegPair.Reg.SourceRange);
       CSInfo.emplace_back(Reg, ArgRegPair.ArgNo);
     }

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

+;; Test enumeration of entry basic blocks when using GlobalISel.
+;; When using other instruction selectors entry blocks will start
+;; from 0, in case of GlobalISel they start from 1.
+; RUN: llc -emit-call-site-info -global-isel %s -stop-after=finalize-isel -o %t.mir
+; RUN: cat %t.mir | FileCheck %s
+; CHECK: name: fn1
+; CHECK-NOT: bb.0.entry
+; CHECK: name: fn3
+; CHECK-NOT: bb.0.entry
+; CHECK: name: fn2
+; CHECK-NOT: bb.0.entry
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @fn1(i32 %a) local_unnamed_addr {
+entry:
+  %c = add i32 %a, 2
+  ret i32 %c
+}
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i32 @fn3(i32 %a, i32 %b, i32 %c) local_unnamed_addr {
+entry:
+  %d = add i32 %a, %b
+  %e = mul i32 %d, %c
+  %f = call i32 @fn1(i32 %e)
+  ret i32 %f
+}
+
+; Function Attrs: noinline nounwind uwtable
+define dso_local i64 @fn2(i32 %a, i32 %b, i32 %c) local_unnamed_addr {
+entry:
+  %call = call i32 @fn1(i32 5)
+  %add = mul i32 %a, 3
+  %sub = sub i32 %add, %b
+  %add2 = add i32 %sub, %c
+  %conv4 = sext i32 %add2 to i64
+  %call2 = call i32 @fn3(i32 5, i32 %add2, i32 %add2)
+  ret i64 %conv4
+}