* BUG_ON and gcc don't mix
@ 2013-08-20 2:37 Anton Blanchard
2013-08-20 4:24 ` Benjamin Herrenschmidt
2013-08-20 4:32 ` Alan Modra
0 siblings, 2 replies; 6+ messages in thread
From: Anton Blanchard @ 2013-08-20 2:37 UTC (permalink / raw)
To: benh, paulus, Alan Modra, wschmidt; +Cc: linuxppc-dev
Hi,
I noticed our BUG_ON macros were taking a large number of instructions.
I've built a testcase to analyse it:
#if defined(ASMBUG)
#define BUG_ON(x) do { \
__asm__ __volatile__("tdnei %0,0\n" : : "r" ((long)(x))); \
} while (0)
#elif defined(BUILTIN)
#define BUG_ON(x) do { \
if (x) \
__builtin_trap(); \
} while (0)
#else
#define BUG_ON(x) do { \
if (x) { \
__asm__ __volatile__("twi 31,0,0\n"); \
__builtin_unreachable(); \
} \
} while (0)
#endif
int foo(unsigned int *bar)
{
unsigned int holder_cpu;
holder_cpu = *bar & 0xffff;
BUG_ON(holder_cpu >= 32);
return 1;
}
3 versions. First our current upstream behaviour (-DASMBUG):
0: 00 00 23 a1 lhz r9,0(r3)
4: 1f 00 89 2b cmplwi cr7,r9,31
8: 26 00 20 7d mfcr r9
c: fe f7 29 55 rlwinm r9,r9,30,31,31
10: 00 00 09 0b tdnei r9,0
14: 01 00 60 38 li r3,1
18: 20 00 80 4e blr
What a load of work. We do the compare, then pull it out of the
condition register and do some more work. We are trying to help gcc
but it seems to be backfiring. Let's try doing a simple version in c:
0: 00 00 23 a1 lhz r9,0(r3)
4: 1f 00 89 2b cmplwi cr7,r9,31
8: 0c 00 9d 41 bgt cr7,14
c: 01 00 60 38 li r3,1
10: 20 00 80 4e blr
14: 00 00 e0 0f twui r0,0
Better, we branch out of line to do the trap. But if we could do a
conditional trap properly then we should be able to do even better
(-DBUILTIN):
0: 00 00 23 a1 lhz r9,0(r3)
4: 01 00 60 38 li r3,1
8: 20 00 a9 0c twlgei r9,32
c: 20 00 80 4e blr
Nice! I remember chasing this down before and the issue is we need the
address of the trap instruction for our bug exception table. Maybe
we need a gcc builtin in which we can get a label on the trap
instruction. Would that be possible?
Anton
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: BUG_ON and gcc don't mix
2013-08-20 2:37 BUG_ON and gcc don't mix Anton Blanchard
@ 2013-08-20 4:24 ` Benjamin Herrenschmidt
2013-08-20 4:32 ` Alan Modra
1 sibling, 0 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2013-08-20 4:24 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev, paulus, wschmidt, Alan Modra
On Tue, 2013-08-20 at 12:37 +1000, Anton Blanchard wrote:
> 0: 00 00 23 a1 lhz r9,0(r3)
> 4: 01 00 60 38 li r3,1
> 8: 20 00 a9 0c twlgei r9,32
> c: 20 00 80 4e blr
>
> Nice! I remember chasing this down before and the issue is we need the
> address of the trap instruction for our bug exception table. Maybe
> we need a gcc builtin in which we can get a label on the trap
> instruction. Would that be possible?
I suppose we can always just do a label before builtin_trap and have
the BUG code search a few instructions :-)
Cheers,
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: BUG_ON and gcc don't mix
2013-08-20 2:37 BUG_ON and gcc don't mix Anton Blanchard
2013-08-20 4:24 ` Benjamin Herrenschmidt
@ 2013-08-20 4:32 ` Alan Modra
2013-08-20 4:45 ` Alan Modra
1 sibling, 1 reply; 6+ messages in thread
From: Alan Modra @ 2013-08-20 4:32 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev, paulus, wschmidt
On Tue, Aug 20, 2013 at 12:37:50PM +1000, Anton Blanchard wrote:
> address of the trap instruction for our bug exception table. Maybe
> we need a gcc builtin in which we can get a label on the trap
> instruction. Would that be possible?
Not your actual _EMIT_BUG_ENTRY, but something like this ought to work.
The only trick here is not putting anything after __builtin_trap()..
#define BUG_ON(x) do { \
if (x) { \
__asm__ __volatile__ ("\n1:" \
"\t.section __bug_table,\"a\"" \
"\n\t.long 1b" \
"\n\t.previous"); \
__builtin_trap(); \
} \
} while (0)
int foo(unsigned int *bar)
{
unsigned int holder_cpu;
holder_cpu = *bar & 0xffff;
BUG_ON(holder_cpu >= 32);
return 1;
}
--
Alan Modra
Australia Development Lab, IBM
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: BUG_ON and gcc don't mix
2013-08-20 4:32 ` Alan Modra
@ 2013-08-20 4:45 ` Alan Modra
2013-08-20 8:36 ` David Laight
0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2013-08-20 4:45 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev, paulus, wschmidt
On Tue, Aug 20, 2013 at 02:02:11PM +0930, Alan Modra wrote:
> On Tue, Aug 20, 2013 at 12:37:50PM +1000, Anton Blanchard wrote:
> > address of the trap instruction for our bug exception table. Maybe
> > we need a gcc builtin in which we can get a label on the trap
> > instruction. Would that be possible?
>
> Not your actual _EMIT_BUG_ENTRY, but something like this ought to work.
> The only trick here is not putting anything after __builtin_trap()..
Doh! I guess the whole point was to get the condition folded into the
trap, which is foiled by emitting an asm between the condition and
buildin_trap().
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: BUG_ON and gcc don't mix
2013-08-20 4:45 ` Alan Modra
@ 2013-08-20 8:36 ` David Laight
2013-08-20 10:55 ` Anton Blanchard
0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2013-08-20 8:36 UTC (permalink / raw)
To: Alan Modra, Anton Blanchard; +Cc: paulus, linuxppc-dev, wschmidt
> On Tue, Aug 20, 2013 at 02:02:11PM +0930, Alan Modra wrote:
> > On Tue, Aug 20, 2013 at 12:37:50PM +1000, Anton Blanchard wrote:
> > > address of the trap instruction for our bug exception table. Maybe
> > > we need a gcc builtin in which we can get a label on the trap
> > > instruction. Would that be possible?
> >
> > Not your actual _EMIT_BUG_ENTRY, but something like this ought to =
work.
> > The only trick here is not putting anything after __builtin_trap()..
>
> Doh! I guess the whole point was to get the condition folded into the
> trap, which is foiled by emitting an asm between the condition and
> buildin_trap().
I was thinking that you could add the label after the trap and
then use '.long 1b-4'. But you'd have to put the asm outside the
conditional - so that wouldn't work if the condition was more
complicated and the trap had to be out of line.
If the trap is out of line, then it could be a long way from
the surrounding code block - so a label 'nearby' in the C isn't
any use.
With fix sized instructions the .text segment of the object files
could be scanned for trap instructions.
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: BUG_ON and gcc don't mix
2013-08-20 8:36 ` David Laight
@ 2013-08-20 10:55 ` Anton Blanchard
0 siblings, 0 replies; 6+ messages in thread
From: Anton Blanchard @ 2013-08-20 10:55 UTC (permalink / raw)
To: David Laight; +Cc: paulus, linuxppc-dev, wschmidt, Alan Modra
Hi David,
> I was thinking that you could add the label after the trap and
> then use '.long 1b-4'. But you'd have to put the asm outside the
> conditional - so that wouldn't work if the condition was more
> complicated and the trap had to be out of line.
>
> If the trap is out of line, then it could be a long way from
> the surrounding code block - so a label 'nearby' in the C isn't
> any use.
>
> With fix sized instructions the .text segment of the object files
> could be scanned for trap instructions.
I tried something similar a while ago and the 1b-4 trick worked for
95%+ of cases but we did end up with out of line traps. I hacked
something up to look for an example, tagging the start and the end of a
BUG_ON, and seeing if the trap was in fact out of line:
asm volatile("1:\n");
if (x)
__builtin_trap();
asm volatile("2:\n");
And this one popped in arch/powerpc/platforms/powernv/pci-ioda.c:
1:
ld 9,0(29)
rlwinm. 8,9,0,29,30
beq- 0,.L287
2:
...
.L287:
trap
You could follow branches and look for the trap, but I'm sure you could
construct things that would be very hard to automatically parse, eg
multiple back to back BUG_ON()'s. At that point I figured some gcc help
would be nice :)
Anton
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-20 10:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-20 2:37 BUG_ON and gcc don't mix Anton Blanchard
2013-08-20 4:24 ` Benjamin Herrenschmidt
2013-08-20 4:32 ` Alan Modra
2013-08-20 4:45 ` Alan Modra
2013-08-20 8:36 ` David Laight
2013-08-20 10:55 ` Anton Blanchard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).