Re: Should invalid __RTL testcase "startwith" passes emit a warning?
On Tue, Feb 19, 2019 at 3:29 PM Matthew Malcomson
wrote:
>
> Hi there,
>
> I'd like to make handling of the __RTL function testcases where the
> startwith pass name is either invalid, not used for that optimisation
> level, or non-existant more understandable.
>
> Currently a problem with the pass name leaves around state that causes
> the compiler to ICE on other functions.
> If the pass name is invalid or one not used for the current optimisation
> level then "dfinit" is run, but "dfinish" is not, which breaks an
> assertion in the `rest_of_handle_df_finish` function.
> For any of the problems the "*clean_state" pass is not run, which causes
> an ICE on the first C function in the TU.
>
> The ICE's I've seen can be avoided by always running the "*clean_state"
> pass (including if the startwith pass of the function is not specified)
> and by always running the "dfinish" pass if the "dfinit" pass is run and
> I am working on a patch to do this.
>
> Since the function will not emit any code for any of these problems, I
> was wondering whether to emit a -Wunused-function warning pointing to
> the bad name (or to the area where a name should be), since it's
> unlikely to be intended.
> The current behaviour (apart from causing an ICE on other functions) is
> to silently do nothing.
You are supposed to write correct __RTL (or __GIMPLE).
Possibly clearing bad state to not leak to other functions might be a
good idea though.
Richard.
>
> Example for "ICE after a bad name".
>
>
> int
> foo_a ()
> {
>return 200;
> }
>
>
> int __RTL (startwith ("badname")) foo2 ()
> {
> (function "foo2"
>(insn-chain
> (block 2
>(edge-from entry (flags "FALLTHRU"))
>(cnote 3 [bb 2] NOTE_INSN_BASIC_BLOCK)
>(cinsn 101 (set (reg:DI x19) (reg:DI x0)))
>(cinsn 10 (use (reg/i:SI x19)))
>(edge-to exit (flags "FALLTHRU"))
> ) ;; block 2
>) ;; insn-chain
> ) ;; function "foo2"
> }
>
>
> Cheers,
> Matthew
Re: Question regarding constraint usage within inline asm
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 [ ]) 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 [ ])) "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 [ ])
(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
Re: Idea: extend gcc to save C from the hell of intel vector instructions
On 2/18/19, Andrew Pinski wrote: > GCC already has most of this support. See > https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Vector-Extensions.html#Vector-Extensions > > The dot in the typenames are not going to supported though. > > Thanks, > Andrew --what #include files and/or compiler flags are needed to enable this stuff? (Be nice if that doc page said.) My gcc allows me to use, e.g. c = _mm_shuffle_epi8(a, b); (and my code worked!) provided I have done #include #include but if I try to replace that with the nicer (since more portable) c = __builtin_shuffle(a, b); then error: use of unknown builtin '__builtin_shuffle' [-Wimplicit-function-declaration] -- Warren D. Smith http://RangeVoting.org <-- add your endorsement (by clicking "endorse" as 1st step)
Re: Idea: extend gcc to save C from the hell of intel vector instructions
On Wed, 20 Feb 2019, Warren D Smith wrote: > but if I try to replace that with the nicer (since more portable) >c = __builtin_shuffle(a, b); > then > error: use of unknown builtin '__builtin_shuffle' > [-Wimplicit-function-declaration] Most likely you're on OS X and the 'gcc' command actually invokes Clang/LLVM. Clang does not implement this builtin (there's __builtin_shufflevector with a different interface — see Clang documentation for details). Alexander
Re: Question regarding constraint usage within inline asm
On Wed, Feb 20, 2019 at 10:08:07AM -0600, Peter Bergner wrote:
> 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;
> >> }
> > 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 [ ]) 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))
expand already uses only one pseudo:
;; __asm__("blah %0, %1, %2" : "=&r" output : "r" input.0_1, "0" input.0_1);
(insn 7 6 0 (parallel [
(set (reg/v:DI 3 3 [ output ])
(asm_operands:DI ("blah %0, %1, %2") ("=&r") 0 [
(reg:DI 121 [ input.0_1 ]) repeated x2
]
[
(asm_input:DI ("r") test.c:6)
(asm_input:DI ("0") test.c:6)
]
[] test.c:6))
(clobber (reg:SI 76 ca))
]) "test.c":6:3 -1
(nil))
and that is a bad idea. The asmcons pass makes pseudo 121 equal to hard
reg 3, and there is no way to recover from that.
Without the local register asm you get the same pseudo for the output as
well as both inputs, just as bad, but LRA can handle this:
0 Early clobber: reject++
0 Conflict early clobber reload: reject--
alt=0,overall=6,losers=1,rld_nregs=0
Choosing alt 0 in insn 8: (0) =&r (1) r (2) 0
Creating newreg=126 from oldreg=123, assigning class GENERAL_REGS to r126
8: {r123:DI=asm_operands;clobber ca:SI;}
REG_UNUSED ca:SI
Inserting insn reload before:
19: r126:DI=r123:DI
So expand shouldn't do this, but also asmcons should probably be improved
Segher
Re: Question regarding constraint usage within inline asm
On Wed, Feb 20, 2019 at 10:08:07AM -0600, Peter Bergner wrote:
> 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:
I meant by the time you get to assembly.
blah 3, 9, 3
> That said, talking with Segher and Uli offline, they both think the
> inline asm usage in the test case should be legal
Good, it seems we are in agreement. Incidentally, the single pseudo
for the inputs happens even for testcases like
long input;
long
bug (void)
{
register long output /* asm ("r3") */;
asm ("blah %0, %1, %2" : "=r" (output) : "wi" (input), "0" (input));
return output;
}
--
Alan Modra
Australia Development Lab, IBM
Re: Question regarding constraint usage within inline asm
I forgot to say, gcc-6, gcc-7 and gcc-8 handle your original testcase with the register asm just fine. -- Alan Modra Australia Development Lab, IBM
Re: Question regarding constraint usage within inline asm
On 2/20/19 4:19 PM, Alan Modra wrote:
> I forgot to say, gcc-6, gcc-7 and gcc-8 handle your original testcase
> with the register asm just fine.
Yes, because they don't have my IRA and LRA patches that exposed this
problem. I would say they were buggy for not complaining and silently
spilling a hard register in the case where we used asm reg("...").
Peter
Re: Question regarding constraint usage within inline asm
On 2/20/19 4:04 PM, Alan Modra wrote:
> On Wed, Feb 20, 2019 at 10:08:07AM -0600, Peter Bergner wrote:
>> On 2/19/19 9:09 PM, Alan Modra wrote:
>> That said, talking with Segher and Uli offline, they both think the
>> inline asm usage in the test case should be legal
>
> Good, it seems we are in agreement. Incidentally, the single pseudo
> for the inputs happens even for testcases like
>
> long input;
> long
> bug (void)
> {
> register long output /* asm ("r3") */;
> asm ("blah %0, %1, %2" : "=r" (output) : "wi" (input), "0" (input));
> return output;
> }
This is a different problem than I'm fixing, but you are correct that
asmcons shouldn't replace operand %1 since it has a non-compatible
constraint than the output operand. In this case, it's probably "ok"
to spill even though it's a hard register, because it doesn't match
the regclass it is supposed to have. I'm not sure how important
this is to fix. It can also imagine that this would be hard to
handle, since we'd have to call into the backend to see whether the
two constraints are compatible and with the overlap between different
constraints, that could be very very messy!
Peter
Re: Question regarding constraint usage within inline asm
On Wed, Feb 20, 2019 at 08:57:52PM -0600, Peter Bergner wrote:
> On 2/20/19 4:19 PM, Alan Modra wrote:
> > I forgot to say, gcc-6, gcc-7 and gcc-8 handle your original testcase
> > with the register asm just fine.
>
> Yes, because they don't have my IRA and LRA patches that exposed this
> problem. I would say they were buggy for not complaining and silently
> spilling a hard register in the case where we used asm reg("...").
I don't follow your reasoning. It seems to me that giving some
variable a register asm doesn't mean that the value of that variable
can't appear in some other register. An obvious example is when
passing that variable to a function.
So why shouldn't a hard reg be reloaded in order to satisfy
incompatible constraints?
--
Alan Modra
Australia Development Lab, IBM
