diff mbox series

aarch64: Fix undefinedness while testing the J constraint [PR100204]

Message ID 20240125195712.410550-1-quic_apinski@quicinc.com
State Accepted
Headers
Series aarch64: Fix undefinedness while testing the J constraint [PR100204] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski (QUIC) Jan. 25, 2024, 7:57 p.m. UTC 
  The J constraint can invoke undefined behavior due to it taking the
negative of the ival if ival was HWI_MIN. The fix is simple as casting
to `unsigned HOST_WIDE_INT` before doing the negative of it. This
does that.

Committed as obvious after build/test for aarch64-linux-gnu.

gcc/ChangeLog:

	PR target/100204
	* config/aarch64/constraints.md (J): Cast to `unsigned HOST_WIDE_INT`
	before taking the negative of it.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/config/aarch64/constraints.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alex Coplan Jan. 26, 2024, 10:07 a.m. UTC | #1 
On 25/01/2024 11:57, Andrew Pinski wrote:
> The J constraint can invoke undefined behavior due to it taking the
> negative of the ival if ival was HWI_MIN. The fix is simple as casting
> to `unsigned HOST_WIDE_INT` before doing the negative of it. This
> does that.

Thanks for doing this.

> 
> Committed as obvious after build/test for aarch64-linux-gnu.
> 
> gcc/ChangeLog:
> 
> 	PR target/100204
> 	* config/aarch64/constraints.md (J): Cast to `unsigned HOST_WIDE_INT`
> 	before taking the negative of it.
> 
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/config/aarch64/constraints.md | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
> index 8566befd727..a2569cea510 100644
> --- a/gcc/config/aarch64/constraints.md
> +++ b/gcc/config/aarch64/constraints.md
> @@ -118,7 +118,7 @@ (define_constraint "Uat"
>  (define_constraint "J"
>   "A constant that can be used with a SUB operation (once negated)."
>   (and (match_code "const_int")
> -      (match_test "aarch64_uimm12_shift (-ival)")))
> +      (match_test "aarch64_uimm12_shift (- (unsigned HOST_WIDE_INT) ival)")))

Sorry for the nitpick, but: I don't think we want a space after the unary -
here (at least according to https://gcc.gnu.org/codingconventions.html).

Alex

>  
>  ;; We can't use the mode of a CONST_INT to determine the context in
>  ;; which it is being used, so we must have a separate constraint for
> -- 
> 2.39.3
>
diff mbox series

Patch

diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
index 8566befd727..a2569cea510 100644
--- a/gcc/config/aarch64/constraints.md
+++ b/gcc/config/aarch64/constraints.md
@@ -118,7 +118,7 @@  (define_constraint "Uat"
 (define_constraint "J"
  "A constant that can be used with a SUB operation (once negated)."
  (and (match_code "const_int")
-      (match_test "aarch64_uimm12_shift (-ival)")))
+      (match_test "aarch64_uimm12_shift (- (unsigned HOST_WIDE_INT) ival)")))
 
 ;; We can't use the mode of a CONST_INT to determine the context in
 ;; which it is being used, so we must have a separate constraint for