[llvm-commits] [PATCH] Machine code verifier pass
Evan Cheng
evan.cheng at apple.com
Mon May 11 18:07:28 PDT 2009
On May 11, 2009, at 1:45 PM, Jakob Stoklund Olesen wrote:
>
> On 11/05/2009, at 20.43, Evan Cheng wrote:
>> + const MachineBasicBlock *MBB;
>> + BBInfo *MBBInfo;
>> + const MachineInstr *MI;
>> + const MachineOperand *MO;
>>
>> I find it strange that these are class ivars. Can you change them to
>> local variables and parameters?
>
> They provide context information to the report() messages, but you are
> right, it is a bit weird. Functions declaring local variables of the
> same name doesn't help either. I'll change it.
Thanks.
>
>> +void
>> +MachineVerifier::calcMaxRegsPassed()
>> +{
>> + // First push live-out regs to successors' vregsPassed. Remember
>> the MBBs that
>> + // have any vregsPassed.
>> + DenseSet<const MachineBasicBlock*> todo;
>> + for (MachineFunction::const_iterator MFI = MF->begin(), MFE = MF-
>>> end();
>> + MFI != MFE; ++MFI) {
>> + const MachineBasicBlock &MBB(*MFI);
>> + BBInfo &MInfo = MBBInfoMap[&MBB];
>> + for (MachineBasicBlock::const_succ_iterator SuI =
>> MBB.succ_begin(),
>> + SuE = MBB.succ_end(); SuI != SuE; ++SuI) {
>> + BBInfo &SInfo = MBBInfoMap[*SuI];
>> + if (SInfo.addPassed(MInfo.regsLiveOut))
>> + todo.insert(*SuI);
>> + }
>> + }
>> +
>> + // Iteratively push vregsPassed to successors until convergence.
>> + while (!todo.empty()) {
>>
>> I don't think the order of iteration of DenseSet is deterministic.
>
> That is true, but it shouldn't matter. The algorithm will converge to
> the same final state regardless of the iteration order.
Ok.
>
>> + // If any regs removed, we need to recheck successors
>>
>> Missing a period at the end of the sentence.
>
> Fixed.
>
>> Have you had a chance running this through some code?
>
> Yes, I am using it with all my Blackfin test cases. I have also tried
> enabling it permanently when running the CodeGen test suite.
>
> CodeGen/ARM reveals that the verifier does not handle PHI instructions
> properly. I will have to rework this! I also get lots of these errors:
>
> *** Bad machine code: Illegal physical register for instruction ***
> - function: f1
> - basic block: ID#0
> - instruction: %SP<def> = tADDspi %SP, 1
> - operand 0: %SP<def>
> SP is not a tGPR register.
>
> This is not a real error - the thumb back-end rewrites these
> instructions at a later stage, I think.
Hrm. I would think tADDspi should be changed to target GPR instead of
tGPR since it's supposed to update SP. Jim, is there a reason why
that's not the case?
How about x86? :-)
>
>
>
>
> Evan, thanks for reviewing. I will implement your suggestions, and I
> will also need do deal with PHI nodes and EH landing pads properly.
> Then I'll submit a new patch.
Great. Thanks.
Evan
>
> /jakob
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list