[PATCH] D88667: [GlobalISel] Change asserting conditions when initializing call site info

Mateja Marjanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 8 08:13:36 PDT 2020


matejam added inline comments.


================
Comment at: llvm/lib/CodeGen/MIRParser/MIRParser.cpp:372-373
+    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)
----------------
djtodoro wrote:
> arsenm wrote:
> > 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
> 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?
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())".


================
Comment at: llvm/test/CodeGen/X86/globalisel-entry-block-enumeration.ll:2-3
+;; 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
----------------
arsenm wrote:
> A pure input MIR test in test/CodeGen/MIR would be preferable
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88667/new/

https://reviews.llvm.org/D88667



More information about the llvm-commits mailing list