linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Anton Blanchard <anton@samba.org>
To: benh@kernel.crashing.org, paulus@samba.org,
	Alan Modra <amodra@gmail.com>,
	wschmidt@us.ibm.com
Cc: linuxppc-dev@lists.ozlabs.org
Subject: BUG_ON and gcc don't mix
Date: Tue, 20 Aug 2013 12:37:50 +1000	[thread overview]
Message-ID: <20130820123750.7353f2f1@kryten> (raw)


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

             reply	other threads:[~2013-08-20  2:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-20  2:37 Anton Blanchard [this message]
2013-08-20  4:24 ` BUG_ON and gcc don't mix 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130820123750.7353f2f1@kryten \
    --to=anton@samba.org \
    --cc=amodra@gmail.com \
    --cc=benh@kernel.crashing.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulus@samba.org \
    --cc=wschmidt@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).