Jakob,
The intention was to identify code that may have been converted from
the old style a little too quickly. I wanted to avoid bugs from a
global s/setIsInsideBundle/bundleWithPred/g search and replace.
This is a good intent. Maybe a bit temporal but sound nevertheless.
finalizeBundle is calling 'MIBundleBuilder Bundle(MBB, FirstMI,
LastMI)' which ought to work with pre-bundled instructions.
Not exactly. Let me illustrate couple cases here (for illustration purposes
"^" means "isBundledWithPred()" and "v" means "isBundledWithSucc()"):
I have the following (existing) bundle for which I want to regenerate the
bundle header (You see that %R17 is not currently in the def list for the
bundle header).
v BUNDLE %P3<imp-def>, %R29<imp-use>, %D8<imp-use,kill>,
%D9<imp-use,kill>, %R6<imp-use>
*^v STrid_indexed %R29, 80, %D8<kill>; mem:ST8[FixedStack2]
*^v STrid_indexed %R29, 72, %D9<kill>; mem:ST8[FixedStack3]
*^v %P3<def> = CMPEQri %R6, 0
*^ %R17<def> = TFR_cdnNotPt %P3<internal>, %R1
v BUNDLE %R29<imp-use>, %D10<imp-use,kill>, %R7<imp-use>, %D6<imp-use>
(next bundle).
finalizeBundle() is called with:
FirstMI == STrid_indexed %R29, 80, %D8<kill>; mem:ST8[FixedStack2]
LastMI == BUNDLE %R29<imp-use>, %D10<imp-use,kill>, %R7<imp-use>,
%D6<imp-use>
From here this assert is triggered:
void MachineInstr::bundleWithSucc() {
assert(!isBundledWithSucc() && "MI is already bundled with its
successor"); <<<<<<<<<<<<<<<<<<<<<<<<
setFlag(BundledSucc);
MachineBasicBlock::instr_iterator Succ = this;
++Succ;
assert(!Succ->isBundledWithPred() && "Inconsistent bundle flags");
Succ->setFlag(BundledPred);
}
Here is the call stack:
#3 ... in llvm::MachineInstr::bundleWithSucc (this=0x4c6aa20) at
...lib/CodeGen/MachineInstr.cpp:882
#4 ... in llvm::MIBundleBuilder::insert (this=0x7fffffff7dc0, I=...,
MI=0x4c6aa20) at ...include/llvm/CodeGen/MachineInstrBuilder.h:417
#5 ... in llvm::MIBundleBuilder::prepend (this=0x7fffffff7dc0,
MI=0x4c6aa20) at ...include/llvm/CodeGen/MachineInstrBuilder.h:435
#6 ... in llvm::finalizeBundle (MBB=..., FirstMI=..., LastMI=...) at
...lib/CodeGen/MachineInstrBundle.cpp:112
Let me give you another example though. I have the following existing
bundle:
v BUNDLE %R0<imp-def,dead>, %PC<imp-def>, %R18<imp-use>
*^v %R0<def> = AND_ri %R18, 128
*^ JMP_EQriPnt_nv_V4 %R0<kill,internal>, 0, <BB#2>, %PC<imp-def>
I need to move an instruction into this bundle from another location and
insert it _between_ AND_ri and JMP_EQriPnt_nv_V4. I use MBB->splice(...) to
do that. Let's pretend that moved instruction was not bundled. New
instruction is pointed to by MachineBasicBlock::instr_iterator MII. New
bundle right after splice is:
v BUNDLE %R0<imp-def,dead>, %PC<imp-def>, %R18<imp-use>
*^v %R0<def> = AND_ri %R18, 128
* %R3<def> = HexagonEXTRACTU_ri_Inst %R18, 4, 3 <<<<<<<<<<<<<<<<<<<
MII
*^ JMP_EQriPnt_nv_V4 %R0<kill,internal>, 0, <BB#2>, %PC<imp-def>
Now I have to integrate MII into the new bundle. As I do this:
MII->bundleWithPred();
I run into this assert:
void MachineInstr::bundleWithPred() {
assert(!isBundledWithPred() && "MI is already bundled with its
predecessor");
setFlag(BundledPred);
MachineBasicBlock::instr_iterator Pred = this;
--Pred;
assert(!Pred->isBundledWithSucc() && "Inconsistent bundle flags");
<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Pred->setFlag(BundledSucc);
}
Now think about what I need to do for all possible cases when original
instruction was previously bundled and that bundle also needs updating....
and on top of that almost every time I call bundleWith*() I have to guard it
with the check for isBundledWith*(). The code looks rather ugly at that
point...
...and if I start dissolve and reconstruct bundles every time I need to
manipulate them, I think original intent becomes a bit overly
constraining...
Sergei
...but this case is trivial and I am OK with it. What is more