[llvm-commits] [PATCH] Machine code verifier pass

[Evan Cheng]

On May 12, 2009, at 1:32 PM, Jakob Stoklund Olesen wrote:

>
> On 12/05/2009, at 21.32, Evan Cheng wrote:
>
>> The double def is also a bug.
>
> There are lots of physical register double defs when using sub- 
> registers, I think they are introduced by the coalescer, but I am  
> not sure. I have disabled the phys-double-def check for now.

There shouldn't be any *real* double defs. However, a def of EAX  
followed by def of a sub-register, e.g. AX, is totally legal.

>
> Attached is take two of the machine code verifier pass. These are  
> the changes:

Looks good. But I don't have a stylistic nitpick:

+      }
+      else {
+        // TwoAddress instr modyfying a reg treated as kill+def.
+        unsigned defIdx=0;
+        if (MI->isRegTiedToDefOperand(MONum, &defIdx) &&
+            MI->getOperand(defIdx).getReg() == Reg)
+          regsKilled.push_back(Reg);
+      }

llvm uses this style instead:
} else {

Also, if check for "MI->getOperand(defIdx).getReg() == Reg" is cheaper  
than isRegTiedToDefOperand, so it should go first.

>
> - Change transient class ivars to method arguments as requested.  
> This is much better.
>
> - Calculate MBB reachability and exclude unreachable MBBs from some  
> checks. There are sometimes dead MBBs with bad live register  
> information.
>
> - Don't verify physical live-in lists for EH landing pads. Unwind  
> creates live registers from thin air.
>
> - Don't require PHI operands to be live-in from all predecessors.  
> (Duh)
>
> - Add dedicated check for PHI operands. Check needed predecessor  
> live-outs individually, and require a PHI operand pair for each  
> predecessor. There are many PHI instructions referring to MBBs that  
> are not predecessors. That looks suspicious, but I am allowing it.
>
> - Check for the environment variable LLVM_VERIFY_MACHINEINSTRS. When  
> it is defined, enable the verifier globally, append output to a file  
> instead of cerr, and don't abort on errors. This means you can say:
>
> $ LLVM_VERIFY_MACHINEINSTRS=verify.out make check
> $ find . -name verify.out -not -empty | xargs fgrep -c '*** Bad  
> machine code'
> ./test/CodeGen/Alpha/Output/verify.out:285
> ./test/CodeGen/ARM/Output/verify.out:1289
> ./test/CodeGen/CellSPU/Output/verify.out:560
> ./test/CodeGen/Generic/Output/verify.out:46
> ./test/CodeGen/IA64/Output/verify.out:26
> ./test/CodeGen/Mips/Output/verify.out:6
> ./test/CodeGen/PowerPC/Output/verify.out:10403
> ./test/CodeGen/X86/Output/verify.out:2047
>
>
> I am using MachineInstr::isRegTiedToDefOperand(), and that is  
> causing asserts on some PowerPC tests because  
> InlineAsm::isUseOperandTiedToDef() is returning an invalid operand  
> number.
>
> tied-badval.patch is a workaround. It is not a fix.

I'll take a look at that. It can be fixed after the verifier goes in.

Thanks,

Evan
>
>
> <machine-verifier.patch><tied- 
> badval.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits