Patchwork [gomp4] mangle linear step of 1 with just 'l'

login
register
mail settings
Submitter Aldy Hernandez
Date Nov. 2, 2013, 1:25 p.m.
Message ID <5274FD48.3060303@redhat.com>
Download mbox | patch
Permalink /patch/287982/
State New
Headers show

Comments

Aldy Hernandez - Nov. 2, 2013, 1:25 p.m.
Hi Jakub.

Your patch mangling negative linear steps caused a regression in 
simd-clones-2.c.  Well, it already had a failure, but now it has two :).

The problem is that AFAIU, a linear step of 1, is mangled with just 'l', 
not 'l1'.

I am not completely sure of this, and was hoping Balaji could clear this 
up, but on page 7 of the Intel vector ABI document, the example for:

__declspec(vector(uniform(a), aligned(a:32), linear(k:1)))
extern float setArray(float *a, float x, int k)

...is mangled as _ZGVxN4ua32vl_setArray, and in the subsequent 
explanatory paragraph, the document specifically says:

	“l” indicates linear(k:1) – k is a linear variable whose stride is 1.

However, since the spec itself says nothing about a default linear 
stride 1, I don't know whether this is an oversight in the BNF grammar 
or a typo in the example.  Balaji?

If a linear step of 1 is mangled as 'l' as the example suggests, then 
I'd like to commit the following patch.

Aldy
gcc/ChangeLog.gomp
	* omp-low.c (simd_clone_mangle): Linear step of 1 is mangled as
	'l'.
Jakub Jelinek - Nov. 2, 2013, 3:50 p.m.
On Sat, Nov 02, 2013 at 08:25:28AM -0500, Aldy Hernandez wrote:
> Your patch mangling negative linear steps caused a regression in
> simd-clones-2.c.  Well, it already had a failure, but now it has two
> :).
> 
> The problem is that AFAIU, a linear step of 1, is mangled with just
> 'l', not 'l1'.
> 
> I am not completely sure of this, and was hoping Balaji could clear
> this up, but on page 7 of the Intel vector ABI document, the example
> for:
> 
> __declspec(vector(uniform(a), aligned(a:32), linear(k:1)))
> extern float setArray(float *a, float x, int k)
> 
> ...is mangled as _ZGVxN4ua32vl_setArray, and in the subsequent
> explanatory paragraph, the document specifically says:
> 
> 	“l” indicates linear(k:1) – k is a linear variable whose stride is 1.
> 
> However, since the spec itself says nothing about a default linear
> stride 1, I don't know whether this is an oversight in the BNF
> grammar or a typo in the example.  Balaji?

Ah, I was reading just the grammar and it didn't look like the number was
optional.

BTW, as I said on IRC yesterday, for say:
#pragma omp declare simd simdlen(8) notinbranch
int foo (int a, int b);
in the 'x' ISA (i.e. SSE2), we are supposed to pass it as
typedef int V __attribute__((vector_size (16)));
foo (V a.1, V a.2, V b.1, V b.2)
which probably isn't that hard for arguments, but are supposed to return
also the return value in two V registers (%xmm0/%xmm1).  We can't pass that
as vector(8) int, so perhaps pretend the return value is ARRAY_TYPE of
2 vector(4) ints and have some special code in the backend to pass that
return value in two xmm registers.  Similarly for 4 and 8.

	Jakub
Aldy Hernandez - Nov. 6, 2013, 10:40 p.m.
On 11/02/13 09:50, Jakub Jelinek wrote:
> On Sat, Nov 02, 2013 at 08:25:28AM -0500, Aldy Hernandez wrote:
>> Your patch mangling negative linear steps caused a regression in
>> simd-clones-2.c.  Well, it already had a failure, but now it has two
>> :).
>>
>> The problem is that AFAIU, a linear step of 1, is mangled with just
>> 'l', not 'l1'.
>>
>> I am not completely sure of this, and was hoping Balaji could clear
>> this up, but on page 7 of the Intel vector ABI document, the example
>> for:
>>
>> __declspec(vector(uniform(a), aligned(a:32), linear(k:1)))
>> extern float setArray(float *a, float x, int k)
>>
>> ...is mangled as _ZGVxN4ua32vl_setArray, and in the subsequent
>> explanatory paragraph, the document specifically says:
>>
>> 	“l” indicates linear(k:1) – k is a linear variable whose stride is 1.
>>
>> However, since the spec itself says nothing about a default linear
>> stride 1, I don't know whether this is an oversight in the BNF
>> grammar or a typo in the example.  Balaji?
>
> Ah, I was reading just the grammar and it didn't look like the number was
> optional.

Balaji has confirmed that a linear step of 1 is mangled as just 'l'.  He 
has also confirmed that icc mangles it this way.

I have committed the patch to the branch.

Aldy

Patch

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index d30fb17..e895cca 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -10825,9 +10825,9 @@  simd_clone_mangle (struct cgraph_node *old_node, struct cgraph_node *new_node)
 	{
 	  gcc_assert (arg.linear_step != 0);
 	  pp_character (&pp, 'l');
-	  if (arg.linear_step > 0)
+	  if (arg.linear_step > 1)
 	    pp_unsigned_wide_integer (&pp, arg.linear_step);
-	  else
+	  else if (arg.linear_step < 0)
 	    {
 	      pp_character (&pp, 'n');
 	      pp_unsigned_wide_integer (&pp, (-(unsigned HOST_WIDE_INT)