Skip to content

Commit

Permalink
aarch64: Don't emit invalid zero/sign-extend syntax
Browse files Browse the repository at this point in the history
Given the following C function:

double *f(double *p, unsigned x)
{
    return p + x;
}

prior to this patch, GCC at -O2 would generate:

f:
        add     x0, x0, x1, uxtw 3
        ret

but this add instruction uses architecturally-invalid syntax: the width
of the third operand conflicts with the width of the extension
specifier. The third operand is only permitted to be an x register when
the extension specifier is (u|s)xtx.

This instruction, and analogous insns for adds, sub, subs, and cmp, are
rejected by clang, but accepted by binutils. Assembling and
disassembling such an insn with binutils gives the architecturally-valid
version in the disassembly:

   0:   8b214c00        add     x0, x0, w1, uxtw #3

This patch fixes several patterns in the AArch64 backend to use the
standard syntax as specified in the Arm ARM such that GCC's output can
be assembled by assemblers other than GAS.

---

gcc/ChangeLog:

	* config/aarch64/aarch64.md
	(*adds_<optab><ALLX:mode>_<GPI:mode>): Ensure extended operand
	agrees with width of extension specifier.
	(*subs_<optab><ALLX:mode>_<GPI:mode>): Likewise.
	(*adds_<optab><ALLX:mode>_shift_<GPI:mode>): Likewise.
	(*subs_<optab><ALLX:mode>_shift_<GPI:mode>): Likewise.
	(*add_<optab><ALLX:mode>_<GPI:mode>): Likewise.
	(*add_<optab><ALLX:mode>_shft_<GPI:mode>): Likewise.
	(*add_uxt<mode>_shift2): Likewise.
	(*sub_<optab><ALLX:mode>_<GPI:mode>): Likewise.
	(*sub_<optab><ALLX:mode>_shft_<GPI:mode>): Likewise.
	(*sub_uxt<mode>_shift2): Likewise.
	(*cmp_swp_<optab><ALLX:mode>_reg<GPI:mode>): Likewise.
	(*cmp_swp_<optab><ALLX:mode>_shft_<GPI:mode>): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.target/aarch64/adds3.c: Fix test w.r.t. new syntax.
	* gcc.target/aarch64/cmp.c: Likewise.
	* gcc.target/aarch64/subs3.c: Likewise.
	* gcc.target/aarch64/subsp.c: Likewise.
	* gcc.target/aarch64/extend-syntax.c: New test.
  • Loading branch information
acoplan-arm committed Sep 7, 2020
1 parent 931832a commit d4febc7
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 16 deletions.
24 changes: 12 additions & 12 deletions gcc/config/aarch64/aarch64.md
Original file line number Diff line number Diff line change
Expand Up @@ -2383,7 +2383,7 @@
(set (match_operand:GPI 0 "register_operand" "=r")
(plus:GPI (ANY_EXTEND:GPI (match_dup 1)) (match_dup 2)))]
""
"adds\\t%<GPI:w>0, %<GPI:w>2, %<GPI:w>1, <su>xt<ALLX:size>"
"adds\\t%<GPI:w>0, %<GPI:w>2, %w1, <su>xt<ALLX:size>"
[(set_attr "type" "alus_ext")]
)

Expand All @@ -2397,7 +2397,7 @@
(set (match_operand:GPI 0 "register_operand" "=r")
(minus:GPI (match_dup 1) (ANY_EXTEND:GPI (match_dup 2))))]
""
"subs\\t%<GPI:w>0, %<GPI:w>1, %<GPI:w>2, <su>xt<ALLX:size>"
"subs\\t%<GPI:w>0, %<GPI:w>1, %w2, <su>xt<ALLX:size>"
[(set_attr "type" "alus_ext")]
)

Expand All @@ -2415,7 +2415,7 @@
(match_dup 2))
(match_dup 3)))]
""
"adds\\t%<GPI:w>0, %<GPI:w>3, %<GPI:w>1, <su>xt<ALLX:size> %2"
"adds\\t%<GPI:w>0, %<GPI:w>3, %w1, <su>xt<ALLX:size> %2"
[(set_attr "type" "alus_ext")]
)

Expand All @@ -2433,7 +2433,7 @@
(ashift:GPI (ANY_EXTEND:GPI (match_dup 2))
(match_dup 3))))]
""
"subs\\t%<GPI:w>0, %<GPI:w>1, %<GPI:w>2, <su>xt<ALLX:size> %3"
"subs\\t%<GPI:w>0, %<GPI:w>1, %w2, <su>xt<ALLX:size> %3"
[(set_attr "type" "alus_ext")]
)

Expand Down Expand Up @@ -2549,7 +2549,7 @@
(plus:GPI (ANY_EXTEND:GPI (match_operand:ALLX 1 "register_operand" "r"))
(match_operand:GPI 2 "register_operand" "r")))]
""
"add\\t%<GPI:w>0, %<GPI:w>2, %<GPI:w>1, <su>xt<ALLX:size>"
"add\\t%<GPI:w>0, %<GPI:w>2, %w1, <su>xt<ALLX:size>"
[(set_attr "type" "alu_ext")]
)

Expand All @@ -2571,7 +2571,7 @@
(match_operand 2 "aarch64_imm3" "Ui3"))
(match_operand:GPI 3 "register_operand" "r")))]
""
"add\\t%<GPI:w>0, %<GPI:w>3, %<GPI:w>1, <su>xt<ALLX:size> %2"
"add\\t%<GPI:w>0, %<GPI:w>3, %w1, <su>xt<ALLX:size> %2"
[(set_attr "type" "alu_ext")]
)

Expand Down Expand Up @@ -2819,7 +2819,7 @@
"*
operands[3] = GEN_INT (aarch64_uxt_size (INTVAL(operands[2]),
INTVAL (operands[3])));
return \"add\t%<w>0, %<w>4, %<w>1, uxt%e3 %2\";"
return \"add\t%<w>0, %<w>4, %w1, uxt%e3 %2\";"
[(set_attr "type" "alu_ext")]
)

Expand Down Expand Up @@ -3305,7 +3305,7 @@
(ANY_EXTEND:GPI
(match_operand:ALLX 2 "register_operand" "r"))))]
""
"sub\\t%<GPI:w>0, %<GPI:w>1, %<GPI:w>2, <su>xt<ALLX:size>"
"sub\\t%<GPI:w>0, %<GPI:w>1, %w2, <su>xt<ALLX:size>"
[(set_attr "type" "alu_ext")]
)

Expand All @@ -3328,7 +3328,7 @@
(match_operand:ALLX 2 "register_operand" "r"))
(match_operand 3 "aarch64_imm3" "Ui3"))))]
""
"sub\\t%<GPI:w>0, %<GPI:w>1, %<GPI:w>2, <su>xt<ALLX:size> %3"
"sub\\t%<GPI:w>0, %<GPI:w>1, %w2, <su>xt<ALLX:size> %3"
[(set_attr "type" "alu_ext")]
)

Expand Down Expand Up @@ -3607,7 +3607,7 @@
"*
operands[3] = GEN_INT (aarch64_uxt_size (INTVAL (operands[2]),
INTVAL (operands[3])));
return \"sub\t%<w>0, %<w>4, %<w>1, uxt%e3 %2\";"
return \"sub\t%<w>0, %<w>4, %w1, uxt%e3 %2\";"
[(set_attr "type" "alu_ext")]
)

Expand Down Expand Up @@ -4054,7 +4054,7 @@
(match_operand:ALLX 0 "register_operand" "r"))
(match_operand:GPI 1 "register_operand" "r")))]
""
"cmp\\t%<GPI:w>1, %<GPI:w>0, <su>xt<ALLX:size>"
"cmp\\t%<GPI:w>1, %w0, <su>xt<ALLX:size>"
[(set_attr "type" "alus_ext")]
)

Expand All @@ -4066,7 +4066,7 @@
(match_operand 1 "aarch64_imm3" "Ui3"))
(match_operand:GPI 2 "register_operand" "r")))]
""
"cmp\\t%<GPI:w>2, %<GPI:w>0, <su>xt<ALLX:size> %1"
"cmp\\t%<GPI:w>2, %w0, <su>xt<ALLX:size> %1"
[(set_attr "type" "alus_ext")]
)

Expand Down
2 changes: 1 addition & 1 deletion gcc/testsuite/gcc.target/aarch64/adds3.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,4 @@ int main ()
return 0;
}

/* { dg-final { scan-assembler-times "adds\tx\[0-9\]+, x\[0-9\]+, x\[0-9\]+, sxtw" 2 } } */
/* { dg-final { scan-assembler-times "adds\tx\[0-9\]+, x\[0-9\]+, w\[0-9\]+, sxtw" 2 } } */
3 changes: 2 additions & 1 deletion gcc/testsuite/gcc.target/aarch64/cmp.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,5 @@ cmp_di_test4 (int a, s64 b, s64 c)
}

/* { dg-final { scan-assembler-times "cmp\tw\[0-9\]+, w\[0-9\]+" 2 } } */
/* { dg-final { scan-assembler-times "cmp\tx\[0-9\]+, x\[0-9\]+" 4 } } */
/* { dg-final { scan-assembler-times "cmp\tx\[0-9\]+, x\[0-9\]+" 2 } } */
/* { dg-final { scan-assembler-times "cmp\tx\[0-9\]+, w\[0-9\]+, sxtw" 2 } } */
120 changes: 120 additions & 0 deletions gcc/testsuite/gcc.target/aarch64/extend-syntax.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/* { dg-do compile } */
/* { dg-options "-O2" } */

// Hits *add_uxtdi_shift2 (*add_uxt<mode>_shift2).
/*
** add1:
** add x0, x0, w1, uxtw 3
** ret
*/
unsigned long long *add1(unsigned long long *p, unsigned x)
{
return p + x;
}

// Hits *add_zero_extendsi_di (*add_<optab><ALLX:mode>_<GPI:mode>).
/*
** add2:
** add x0, x0, w1, uxtw
** ret
*/
unsigned long long add2(unsigned long long x, unsigned y)
{
return x + y;
}

// Hits *add_extendsi_shft_di (*add_<optab><ALLX:mode>_shft_<GPI:mode>).
/*
** add3:
** add x0, x0, w1, sxtw 3
** ret
*/
double *add3(double *p, int x)
{
return p + x;
}

// Hits *sub_zero_extendsi_di (*sub_<optab><ALLX:mode>_<GPI:mode>).
/*
** sub1:
** sub x0, x0, w1, uxtw
** ret
*/
unsigned long long sub1(unsigned long long x, unsigned n)
{
return x - n;
}

// Hits *sub_uxtdi_shift2 (*sub_uxt<mode>_shift2).
/*
** sub2:
** sub x0, x0, w1, uxtw 3
** ret
*/
double *sub2(double *x, unsigned n)
{
return x - n;
}

// Hits *sub_extendsi_shft_di (*sub_<optab><ALLX:mode>_shft_<GPI:mode>).
/*
** sub3:
** sub x0, x0, w1, sxtw 3
** ret
*/
double *sub3(double *p, int n)
{
return p - n;
}

// Hits *adds_zero_extendsi_di (*adds_<optab><ALLX:mode>_<GPI:mode>).
int adds1(unsigned long long x, unsigned y)
{
/* { dg-final { scan-assembler-times "adds\tx\[0-9\]+, x\[0-9\]+, w\[0-9\]+, uxtw" 1 } } */
unsigned long long l = x + y;
return !!l;
}

// Hits *adds_extendsi_shift_di (*adds_<optab><ALLX:mode>_shift_<GPI:mode>).
int adds2(long long x, int y)
{
/* { dg-final { scan-assembler-times "adds\tx\[0-9\]+, x\[0-9\]+, w\[0-9\]+, sxtw 3" 1 } } */
long long t = x + ((long long)y << 3);
return !!t;
}

// Hits *subs_zero_extendsi_di (*subs_<optab><ALLX:mode>_<GPI:mode>).
unsigned long long z;
int subs1(unsigned long long x, unsigned y)
{
/* { dg-final { scan-assembler-times "subs\tx\[0-9\]+, x\[0-9\]+, w\[0-9\]+, uxtw" 1 } } */
unsigned long long t = x - y;
z = t;
return !!t;
}

// Hits *subs_extendsi_shift_di (*subs_<optab><ALLX:mode>_shift_<GPI:mode>).
unsigned long long *w;
int subs2(unsigned long long *x, int y)
{
/* { dg-final { scan-assembler-times "subs\tx\[0-9\]+, x\[0-9\]+, w\[0-9\]+, sxtw 3" 1 } } */
unsigned long long *t = x - y;
w = t;
return !!t;
}

// Hits *cmp_swp_zero_extendsi_regdi (*cmp_swp_<optab><ALLX:mode>_reg<GPI:mode>).
int cmp(unsigned long long x, unsigned y)
{
/* { dg-final { scan-assembler-times "cmp\tx\[0-9\]+, w\[0-9\]+, uxtw" 1 } } */
return !!(x - y);
}

// Hits *cmp_swp_extendsi_shft_di (*cmp_swp_<optab><ALLX:mode>_shft_<GPI:mode>).
int cmp2(unsigned long long x, int y)
{
/* { dg-final { scan-assembler-times "cmp\tx\[0-9\]+, w\[0-9\]+, sxtw 3" 1 } } */
return x == ((unsigned long long)y << 3);
}

/* { dg-final { check-function-bodies "**" "" "" } } */
2 changes: 1 addition & 1 deletion gcc/testsuite/gcc.target/aarch64/subs3.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,4 @@ int main ()
return 0;
}

/* { dg-final { scan-assembler-times "subs\tx\[0-9\]+, x\[0-9\]+, x\[0-9\]+, sxtw" 2 } } */
/* { dg-final { scan-assembler-times "subs\tx\[0-9\]+, x\[0-9\]+, w\[0-9\]+, sxtw" 2 } } */
2 changes: 1 addition & 1 deletion gcc/testsuite/gcc.target/aarch64/subsp.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ f2 (int *x, int y)
}

/* { dg-final { scan-assembler "sub\tsp, sp, x\[0-9\]*\n" } } */
/* { dg-final { scan-assembler "sub\tsp, sp, x\[0-9\]*, sxtw 4\n" } } */
/* { dg-final { scan-assembler "sub\tsp, sp, w\[0-9\]*, sxtw 4\n" } } */

0 comments on commit d4febc7

Please sign in to comment.