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.