[llvm-commits] [llvm] r57257 - /llvm/trunk/lib/Target/X86/X86InstrInfo.td

[Dan Gohman]

On Tue, October 7, 2008 11:34 pm, Chris Lattner wrote:
> On Oct 7, 2008, at 2:29 PM, Evan Cheng wrote:
>>>>> We could fix it, but on the other hand it's arguably sloppy for
>>>>> instruction selectors to not precisely describe their physical
>>>>> register uses.
>>>>
>>>> I agree. In this case DVI8r should say it defines AL, AH and uses
>>>> AL,
>>>> AH which makes it clear it reads 2 values and outputs 2.
>>>
>>> No, it reads 1 value and outputs two.  It is not the same as DIV16
>>> and
>>> DIV32.
>>> The way I have it matches Intel docs.
>>
>> Ok. It's good to match what Intel manual says. But that's additional
>> goodness. Previously DIV16r is marked as using AL, AH and the implicit
>> use is copied to AX. While this is not ideal, it's not incorrect.
>> Local liveness cannot mark AX as dead.
>
> To put it another way, it looks like (as Dale said) both changes are
> useful.  The change to make div more closely follow the docs is useful
> on its own merits, but the local RA should still be fixed.

Can you explain why?

If instruction selectors are working properly, this won't matter.
So it's making local RA do more work (read: slowing down -O0!),
and also the MachineInstr DCE used by fast-isel, and as far as I
can tell the only benefit is that it silently covers up bugs
elsewhere.

Would the situation be different if we had a MachineInstr-level
verifier? If it could flag physreg alias abuses, we could be
more confident that we're not missing any other cases.

This is an area where there are a lot of implicit assumptions, and
I'd like to understand the assumptions better.

Thanks,

Dan