On 20.12.2015 23:53, David Wohlferd wrote:
> On 12/20/2015 10:26 AM, Bernd Edlinger wrote:
>> On 19.12.2015 19:54, David Wohlferd wrote:
>>>>>> mep: mep_interrupt_saved_reg looks for ASM_INPUT in the body, and
>>>>>> saves different registers if found.
>>>>> I'm trying to follow this code. A real challenge since I know
>>>>> nothing
>>>>> about mep. But what I see is:
>>>>>
>>>>> - This routine only applies to functions marked as
>>>>> __attribute__((interrupt)).
>>>>> - To correctly generate entry/exit, it walks thru each register
>>>>> (up to
>>>>> FIRST_PSEUDO_REGISTER) to see if it is used by the routine. If there
>>>>> is any basic asm within the routine (regardless of its contents), the
>>>>> register is considered 'in use.'
>>>>>
>>>>> The net result is that every register gets saved/restored by the
>>>>> entry/exit of this routine if it contains basic asm. The reason this
>>>>> is important is that if someone just adds a colon, it would suddenly
>>>>> *not* save/restore all the registers. Depending on what the asm
>>>>> does,
>>>>> this could be bad.
>>>>>
>>>>> Does that sound about right?
>>>>>
>>>> Yes.
>>> Seems like a doc update would be appropriate here too then, if anyone
>>> wanted to volunteer...
>> To confirm this, I built a cross-compiler, but It was difficult, because
>> of pr64402.
>> Yes, a function with __attribute__((interrupt)) spills lots of
>> registers, when it just
>> contains asm(""); but almost nothing, if asm("":); is used. That is
>> remarkable.
>
> So if you use extended and clobber a couple registers asm("":::"r0",
> "r1"), does it spill just those specific registers? That would be cool.
>
Yes, the register names are "$0", "$1" and so on.
I tried
void __attribute__((interrupt))
intf(void)
{
asm("":::"$0", "$1");
}
$0 and $1 got saved, but only $1 got restored !
this is probably a bug. The loop in mep_expand_epilogue
is wrong, and does not restore $0, fixed that and $0 also
got restored.
> I don't write interrupt handlers, and certainly not on mep. But the
> little I know about them says that performance is an important (and
> sometimes critical) characteristic. There would be risk in changing
> this to extended (if you used a register but forgot to clobber it),
> but depending on the interrupt, it could be a nice performance 'win.'
>
> If no one else is prepared to step up to write this, I can. I'm just
> uncomfortable doing so since I can't try it myself. And I feel weird
> writing a patch for mep given that I know nothing about it.
>
> But since Bernd has tried it, maybe something like this added to the
> 'interrupt' attribute on
> https://gcc.gnu.org/onlinedocs/gcc/MeP-Function-Attributes.html
> ---------
> Be aware that if the function contains any basic @code{asm}
> (@pxref{Basic Asm}), all registers (whether referenced in the asm or
> not) will be preserved upon entry and restored upon exiting the
> interrupt. More efficient code can be generated by using extended
> @code{asm} (@pxref{Extended Asm}) and explicitly listing only the
> specific registers that need to be preserved (or none if your asm
> preserves any registers it uses).
> ---------
>
>>>>>> tilegx: They never wrap {} around inline asm. But extended asm, are
>>>>>> handled differently, probably automatically surrounded by braces.
>>>>> I know nothing about tilegx either. I've tried to read the code, and
>>>>> it seems like basic asm does not get 'bundled' while extended
>>>>> might be.
>>>>>
>>>>> Bundling for tilegx (as I understand it) is when you explicitly fill
>>>>> multiple pipelines by doing something like this:
>>>>>
>>>>> { add r3,r4,r5 ; add r7,r8,r9 ; lw r10,r11 }
>>>>>
>>>>> So if you have a basic asm statement, you wouldn't want it
>>>>> automatically bundled by the compiler, since your asm could be more
>>>>> than 3 statements (the max?). Or your asm may do its own
>>>>> bundling. So
>>>>> it makes sense to never output braces when outputting basic asm.
>>>>>
>>>>> I know I'm guessing about what this means, but doesn't it seem like
>>>>> those same limitations would apply to extended? I wonder if this
>>>>> is a
>>>>> bug. I don't see any discussion of bundling (of this sort) in the
>>>>> docs.
>>>>>
>>>> I wold like to build a cross compiler, but currently that target seems
>>>> to be broken. I have to check
>>>> that target anyways, because of my other patch with the memory
>>>> clobbers.
>>>>
>>>> I see in tilegx_asm_output_opcode, that they do somehow automatically
>>>> place braces.
>>>> An asm("pseudo":) has a special meaning, and can be replaced with ""
>>>> or "}".
>>>> However the static buf[100] means that any extended asm string > 95
>>>> characters, invokes
>>>> the gcc_assert in line 5487. In the moment I would _not_ recommend
>>>> changing any asm
>>>> statements without very careful testing.
>>> Yes, the handling of extended asm there is very strange.
>>>
>>> If bundles can only be 3 instructions, then appending an entire
>>> extended asm in a single (already in-use) bundle seems odd. But maybe
>>> that's just because I don't understand tilegx.
>>>
>>> I'm not sure it's just changing basic asm to extended I would be
>>> concerned about. I'd worry about using extended asm at all...
>> I also built a tilegx cross compiler, but that was even more difficult,
>> because of pr68917, which hit me when building the libgcc.
>>
>> I tried a while, but was unable to find any example of an extended asm
>> that gets
>> auto-braces, or which can trigger the mentioned gcc_assert. It looks
>> like, in all
>> cases the bundles are closed already before the extended asm gets
>> scheduled.
>> Probably the middle end already knows that it can't parse the asm
>> string.
>> I see examples of extended asm in the linux /arch/tile tree, and some
>> have
>> braces, some not. But as it seems, they are completely left alone by
>> the expansion.
>
> So what do the docs need to say here? Basic asm never wraps with {},
> but apparently extended doesn't either. So, there's nothing to say?
>
>> You do not have to escape the { and } for extended asm, on this target,
>> using %{ produces even an error.
>
> I believe the only the only target that needs to escape {} is i386,
> since it's the only one that supports dialects (att & intel).
>
>> In deed asm ("pseudo":) expands to nothing, it is
>> probably used as a memory barrier in the .md files.
>
> Yeah, that's what it looked like to me too.
>
>>>> Yes, but please add a test case with some examples where the
>>>> warning is
>>>> expected to be triggered,
>>>> and where it is not.
>>> I have one, but I haven't added the dejagnu stuff to it yet. I'm not
>>> really an expert there and could use some help. Simple things like
>>> where does the file go and what should it be called.
>> Look at gcc/testsuite/c-c++-common/W*.c for examples how to do that.
>> I think, this would be a good place/naming convention for your test case
>> too.
>
> Given that c and cpp use different parsers, I'll want to create tests
> for both (well, same tests, different extensions). Should both
> Wonly-top-basic-asm.c and Wonly-top-basic-asm.cpp go in that
> directory? There aren't any other cpp files there.
>
this is in c-c++-common, so both front ends run these tests.
Bernd.
