diff mbox

PR/50076 make c-c++-common/cxxbitfields-3.c work in Darwin

Message ID 4EE2656C.4080801@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Dec. 9, 2011, 7:45 p.m. UTC 
On 12/09/11 13:19, Jakub Jelinek wrote:
> On Fri, Dec 09, 2011 at 08:17:04PM +0100, Dominique Dhumieres wrote:
>>> +/* { dg-final { scan-assembler "movl.*, (_?var|\\(%)" } } */
>>
>> It works for me too.
>
> Except that when matching just , (% it doesn't test almost anything.
> IMNSHO you should instead just test it { target !fpic } or similar.
> The _? in there is useful though.
>
> 	Jakub

I assume you mean it tests almost anything, in which case I thought the 
source tiny enough to not elicit any more loads that could be matched. 
But the test is annoyingly system dependent.

How about the patch below?

I used nonpic instead of "!fpic", as check_effective_target_fpic tests 
whether -fpic/-fPIC is supported in the driver, which will obviously 
return true even if we're not generating PIC code.  OTOH 
check_effective_target_nonpic tests whether we are generating PIC code 
by checking "#if __PIC__" which I believe is what we want.

Does this work for everyone?
* c-c++-common/cxxbitfields-3.c: Adjust regexp.

Comments

Jack Howarth Dec. 9, 2011, 8:36 p.m. UTC | #1 
On Fri, Dec 09, 2011 at 01:45:48PM -0600, Aldy Hernandez wrote:
> On 12/09/11 13:19, Jakub Jelinek wrote:
>> On Fri, Dec 09, 2011 at 08:17:04PM +0100, Dominique Dhumieres wrote:
>>>> +/* { dg-final { scan-assembler "movl.*, (_?var|\\(%)" } } */
>>>
>>> It works for me too.
>>
>> Except that when matching just , (% it doesn't test almost anything.
>> IMNSHO you should instead just test it { target !fpic } or similar.
>> The _? in there is useful though.
>>
>> 	Jakub
>
> I assume you mean it tests almost anything, in which case I thought the  
> source tiny enough to not elicit any more loads that could be matched.  
> But the test is annoyingly system dependent.
>
> How about the patch below?
>
> I used nonpic instead of "!fpic", as check_effective_target_fpic tests  
> whether -fpic/-fPIC is supported in the driver, which will obviously  
> return true even if we're not generating PIC code.  OTOH  
> check_effective_target_nonpic tests whether we are generating PIC code  
> by checking "#if __PIC__" which I believe is what we want.
>
> Does this work for everyone?

This works for x86_64-apple-darwin11 using...

make -k check RUNTESTFLAGS="dg.exp=cxxbitfields-3.c --target_board=unix'{-m32,-m64}'"

> 	* c-c++-common/cxxbitfields-3.c: Adjust regexp.
> 
> Index: c-c++-common/cxxbitfields-3.c
> ===================================================================
> --- c-c++-common/cxxbitfields-3.c	(revision 182028)
> +++ c-c++-common/cxxbitfields-3.c	(working copy)
> @@ -18,4 +18,4 @@ void setit()
>    var.j = 5;
>  }
>  
> -/* { dg-final { scan-assembler "movl.*, var" } } */
> +/* { dg-final { scan-assembler "movl.*, _?var" { target nonpic } } } */
Iain Sandoe Dec. 9, 2011, 8:56 p.m. UTC | #2 
On 9 Dec 2011, at 19:45, Aldy Hernandez wrote:

> On 12/09/11 13:19, Jakub Jelinek wrote:
>> On Fri, Dec 09, 2011 at 08:17:04PM +0100, Dominique Dhumieres wrote:
>>>> +/* { dg-final { scan-assembler "movl.*, (_?var|\\(%)" } } */
>>>
>>> It works for me too.
>>
>> Except that when matching just , (% it doesn't test almost anything.
>> IMNSHO you should instead just test it { target !fpic } or similar.

Darwin is not the only target that defaults to PIC.

----

ISTM, in this particular instance, the check for
  movl , (%
happens to be enough to detect the case Aldy is interested in (at  
least for the code sequences Darwin generates).

> Does this work for everyone?

... well that "works" for Darwin by not performing the test ..

.. might as well just skip the whole thing and save the cycles on the  
compile ;-)

===

the problem in finding a match s that c and c++ generate different  
sequences
- and the test is in the shared dir
...  otherwise a target-dependent match would work ...

[[**  I'm checking out whether it's feasible to switch off PIC for m64  
Darwin .. so one could just do the test -fno-PIC ..
    ... works for m32 - but PIC is jammed on for x86/m64 ... ]]

Iain

it seems that each case is doing the correct thing ..

==
c / m32

L00000000001$pb:
	movl	L_var$non_lazy_ptr-L00000000001$pb(%ecx), %edx
	movl	(%edx), %eax
	andb	$15, %al
	orl	$80, %eax
	movl	%eax, (%edx)
	ret

c / m64

_setit:
LFB0:
	movq	_var@GOTPCREL(%rip), %rdx
	movl	(%rdx), %eax
	andb	$15, %al
	orl	$80, %eax
	movl	%eax, (%rdx)
	ret

===

c++ / m32
__Z5setitv:
LFB0:
	call	___x86.get_pc_thunk.cx
L00000000001$pb:
	movl	_var-L00000000001$pb(%ecx), %eax
	andb	$15, %al
	orl	$80, %eax
	movl	%eax, _var-L00000000001$pb(%ecx)
	ret

c++ / m64
__Z5setitv:
LFB0:
	movl	_var(%rip), %eax
	andb	$15, %al
	orl	$80, %eax
	movl	%eax, _var(%rip)
	ret
Mike Stump Dec. 11, 2011, 10:51 p.m. UTC | #3 
On Dec 9, 2011, at 11:45 AM, Aldy Hernandez wrote:
> How about the patch below?

I'm fine with whatever you guys come up with...
Aldy Hernandez Dec. 12, 2011, 3:13 p.m. UTC | #4 
On 12/11/11 16:51, Mike Stump wrote:
> On Dec 9, 2011, at 11:45 AM, Aldy Hernandez wrote:
>> How about the patch below?
>
> I'm fine with whatever you guys come up with...

Likewise.  I have no preference.  Whatever gets approved is ok with me.
diff mbox

Patch

Index: c-c++-common/cxxbitfields-3.c
===================================================================
--- c-c++-common/cxxbitfields-3.c	(revision 182028)
+++ c-c++-common/cxxbitfields-3.c	(working copy)
@@ -18,4 +18,4 @@  void setit()
   var.j = 5;
 }
 
-/* { dg-final { scan-assembler "movl.*, var" } } */
+/* { dg-final { scan-assembler "movl.*, _?var" { target nonpic } } } */