Re: [PATCH] PR33661 Fix problem with register asm in templates

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

Just nits, I'll defer the review to a C++ maintainer.

On Thu, Jun 11, 2015 at 03:49:48PM +0200, Andreas Krebbel wrote:

> 2015-06-11  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
> 
> 	PR C++/33661
> 	* gcc/cp/decl.c (cp_finish_decl): Set assembler name for register
> 	asm constructs.
> 	* gcc/cp/pt.c (tsubst_decl): Do not zero out the assembler name
> 	for register asm constructs.

Please avoid gcc/cp/ prefixes in the ChangeLog.

> 2015-06-11  Andreas Krebbel  <krebbel@linux.vnet.ibm.com>
> 
> 	PR C++/33661
> 	* gcc.target/s390/pr33661.cc: New test.
> 	* gcc.target/s390/s390.exp: Run also tests with .cc suffix.

This is undesirable, gcc.target/*/ are C tests.  Won't this e.g.
fail miserably if one tests an --enable-languages=c compiler?
The few target specific C++ tests are simply somewhere in the
testsuite/g++.dg/*/ directories, guarded with { target ... }.

But, if this is a generic problem, I'd say testing it only on s390
is insufficient.  I'd think best would be to just do
#if defined(__x86_64__)
  register unsigned long b __asm__ ("r8") = (unsigned long)&a;
  __asm__ volatile ("reg: %0" : : "r" (b));
#elif defined(__i386__)
  register unsigned long b __asm__ ("ecx") = (unsigned long)&a;
  __asm__ volatile ("reg: %0" : : "r" (b));
#elif defined(__s390__)
  register unsigned long b __asm__ ("r2") = (unsigned long)&a;
  __asm__ volatile ("reg: %0" : : "d" (b));
#elif ...
#else
  unsigned long b = (unsigned long)&a;
#endif
and then just have a bunch of
/* { dg-final { scan-assembler "reg: %r8" { target { { x86_64*-*-* i?86-*-* } && lp64 } } } } */
/* { dg-final { scan-assembler "reg: %ecx" { target { { x86_64*-*-* i?86-*-* } && ia32 } } } } */
/* { dg-final { scan-assembler "reg: %r3" { target s390*-*-* } } } */
Covering just a few of the most popular platforms is surely enough.

> +  register unsigned long b __asm__ ("r3") = (unsigned long)&a;
> +  __asm__ volatile ("reg: %0" : : "d" (b):);

The : before ) is unneeded, isn't it?

	Jakub