On 2/19/19 9:09 PM, Alan Modra wrote:
> On Mon, Feb 18, 2019 at 01:13:31PM -0600, Peter Bergner wrote:
>> long input;
>> long
>> bug (void)
>> {
>> register long output asm ("r3");
>> asm ("blah %0, %1, %2" : "=&r" (output) : "r" (input), "0" (input));
>> return output;
>> }
>>
>> I know an input operand can have a matching constraint associated with
>> an early clobber operand, as there seems to be code that explicitly
>> mentions this scenario. In this case, the user has to manually ensure
>> that the input operand is not clobbered by the early clobber operand.
>> In the case that the input operand uses an "r" constraint, we just
>> ensure that the early clobber operand and the input operand are assigned
>> different registers. My question is, what about the case above where
>> we have the same variable being used for two different inputs with
>> constraints that seem to be incompatible?
>
> Without the asm("r3") gcc will provide your "blah" instruction with
> one register for %0 and %2, and another register for %1. Both
> registers will be initialised with the value of "input".
That's not what I'm seeing. I see one pseudo (123) used for the output
operand and one pseudo (121) used for both input operands. Like so:
(insn 8 6 7 (parallel [
(set (reg:DI 123 [ outputD.2831 ])
(asm_operands:DI ("blah %0, %1, %2") ("=&r") 0 [
(reg/v:DI 121 [ <retval> ]) repeated x2
]
[
(asm_input:DI ("r") bug.i:6)
(asm_input:DI ("0") bug.i:6)
]
[] bug.i:6))
(clobber (reg:SI 76 ca))
]) "bug.i":6:3 -1
(nil))
The only difference between using asm("r3") and not using it is that
pseudo 123 is replaced with hard reg 3 in the output operand. The input
operands use pseudo 121 in both cases. It stays this way up until the
asmcons pass (ie, match_asm_constraints_1) which notices that operand %2
has a matching constraint with operand %0, so it emits a copy before
the asm that writes "input"'s pseudo into "output"'s pseudo and then
rewrites the asm operand %2 to use "output"'s pseudo. But then it goes
ahead and rewrites all other uses of "input"'s pseudos with "output"'s
pseudo, so operand %1 also gets rewritten. So we end up with:
(insn 15 6 8 2 (set (reg:DI 123 [ outputD.2831 ])
(reg/v:DI 121 [ <retval> ])) "bug.i":6:3 -1
(nil))
(insn 8 15 12 2 (parallel [
(set (reg:DI 123 [ outputD.2831 ])
(asm_operands:DI ("blah %0, %1, %2") ("=&r") 0 [
(reg:DI 123 [ outputD.2831 ]) repeated x2
]
[
(asm_input:DI ("r") bug.i:6)
(asm_input:DI ("0") bug.i:6)
]
[] bug.i:6))
(clobber (reg:SI 76 ca))
]) "bug.i":6:3 -1
(expr_list:REG_DEAD (reg/v:DI 121 [ <retval> ])
(expr_list:REG_UNUSED (reg:SI 76 ca)
(nil))))
Now the case above (ie, not using asm("r3")) compiles fine. We assign
pseudo 123 to r3 and LRA's constraint checking code notices that operand
%1 should not be assigned to the same register as the early clobber
output operand, so it spills it. However, when we use asm("r3"),
LRA's constraint checking code again sees that operand %1 shouldn't
have the same register as operand %0, but since it's a preassigned
hard register, it cannot spill it, since there may have been a valid
reason why that particular operand is supposed to be in r3, so we ICE.
I'm not sure we can ever safely spill a hard register.
That said, talking with Segher and Uli offline, they both think the
inline asm usage in the test case should be legal, so that tells me
then that the bug is in the asmcons pass when it rewrites operand %1's
pseudo. It really should check that operand %1's pseudo should not
be updated because it conflicts with the early clobber operand %0.
That would then allow operand %1 and operand %2 to have different
registers. I'll try and prepare a patch that checks for that scenario.
Peter
