public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/115073] New: RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO
@ 2024-05-13 15:02 kito at gcc dot gnu.org
  2024-05-13 15:09 ` [Bug tree-optimization/115073] " jakub at gcc dot gnu.org
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: kito at gcc dot gnu.org @ 2024-05-13 15:02 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115073

            Bug ID: 115073
           Summary: RISC-V: Gimple fold not honor
                    C[LT]Z_DEFINED_VALUE_AT_ZERO
           Product: gcc
           Version: 15.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kito at gcc dot gnu.org
  Target Milestone: ---
            Target: riscv64-unknown-linux-gnu

# What's up?

A loop induction variable initialized from __builtin_ctz (x), and the loop
bound is 32, and increment is one, and GCC turn it into infinite loops when x
is 0.

However RISC-V has defined CTZ_DEFINED_VALUE_AT_ZERO as 32 for SImode, so it's
not UB IMO, but seems like gimple-range-op.cc and match.pd are not handle that.

# Command to reproduce
```
$ riscv64-unknown-elf-gcc -O3 -march=rv64gc_zba_zbb_zbc
```

# Testcase
```c
void f();
void foo(unsigned int id, unsigned int x)
{
    for (unsigned int idx = __builtin_ctz(x); idx < 32; idx++) {
        f();
    }
}
```

# Asm output with comment:

```
foo:
        addi    sp,sp,-32
        sd      s0,16(sp)
        sd      s1,8(sp)
        sd      ra,24(sp)
        ctzw    s0,a1     # s0 is 32 if a1 is 0
        li      s1,32
.L2:
        addiw   s0,s0,1   # thne s0 become 33 here
        call    f
        bne     s0,s1,.L2 # compare with 32, which never terminate
        ld      ra,24(sp)
        ld      s0,16(sp)
        ld      s1,8(sp)
        addi    sp,sp,32
        jr      ra
```


# What I tried?

I try to call CTZ_DEFINED_VALUE_AT_ZERO gimple-range-op.cc but it seems not
help for this test case, and then I found it was screw up at match.pd when ccp
pass.

It applied a CTZ simplifications at match.pd:

```
 (for op (eq ne)
  (simplify
   /* __builtin_ctz (x) == C -> (x & ((1 << (C + 1)) - 1)) == (1 << C).  */
   (op (ctz:s @0) INTEGER_CST@1)
    (with { tree type0 = TREE_TYPE (@0); 
            int prec = TYPE_PRECISION (type0);
          }     
     (if (prec <= MAX_FIXED_MODE_SIZE)
      (if (tree_int_cst_sgn (@1) < 0 || wi::to_widest (@1) >= prec) 
       { constant_boolean_node (op == EQ_EXPR ? false : true, type); }
       (op (bit_and @0 { wide_int_to_tree (type0,
                                           wi::mask (tree_to_uhwi (@1) + 1,
                                                     false, prec)); })
           { wide_int_to_tree (type0,
                               wi::shifted_mask (tree_to_uhwi (@1), 1,
                                                 false, prec)); })))))))
```

Then I found it has checked with CTZ_DEFINED_VALUE_AT_ZERO
(g:75f8900159133ce069ef1d2edf3b67c7bc82e305) untill
g:7383cb56e1170789929201b0dadc156888928fdd, but I realized it because is not
really work well here CLZ_DEFINED_VALUE_AT_ZERO.

So I did some aggressive experiment here: convert __builtin_ctz to IFN_CTZ with
second operand (from C[LT]Z_DEFINED_VALUE_AT_ZERO, ideally), it can work *IF*
backend provide patterns for ctz, but NOT work when backend is not provided, it
could be a problem to RISC-V since ctz is not included in baseline ISA for
RISC-V.

It might be arguable if target didn't have ctz/clz pattern but
C[LT]Z_DEFINED_VALUE_AT_ZERO is provided to backend, so I think middle-end
optimization should still honor with that?

Or another thought is convert that into target macro to resolve the issue
describe in g:75f8900159133ce069ef1d2edf3b67c7bc82e305?


# Aggressive experiment:
```
diff --git a/gcc/c-family/c-gimplify.cc b/gcc/c-family/c-gimplify.cc
index 494da49791d..d84469a6dca 100644
--- a/gcc/c-family/c-gimplify.cc
+++ b/gcc/c-family/c-gimplify.cc
@@ -858,7 +858,16 @@ c_gimplify_expr (tree *expr_p, gimple_seq *pre_p
ATTRIBUTE_UNUSED,
                                  c, CALL_EXPR_ARG (*expr_p, 1));
            return GS_OK;
          }
-       break;
+          if (fndecl && fndecl_built_in_p(fndecl, BUILT_IN_CTZ) &&
+              call_expr_nargs(*expr_p) == 1) {
+            tree a = save_expr(CALL_EXPR_ARG(*expr_p, 0));
+            *expr_p = build_call_expr_internal_loc(
+                EXPR_LOCATION(*expr_p), IFN_CTZ, TREE_TYPE(a), 2, a,
+                build_int_cst(TREE_TYPE(a), 32));
+            return GS_OK;
+          }
+
+        break;
       }

     default:;
```

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug tree-optimization/115073] RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO
  2024-05-13 15:02 [Bug tree-optimization/115073] New: RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO kito at gcc dot gnu.org
@ 2024-05-13 15:09 ` jakub at gcc dot gnu.org
  2024-05-13 15:14 ` pinskia at gcc dot gnu.org
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: jakub at gcc dot gnu.org @ 2024-05-13 15:09 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115073

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That is misunderstanding of what C[LT]Z_DEFINED_VALUE_AT_ZERO is.
The builtin has always UB on zero.
If you want to define it for zero, use x ? __builtin_clz (x) : 32 (and the
like) or
__builtin_clzg (x, 32) in the source.
C[LT]Z_DEFINED_VALUE_AT_ZERO returning 2 then makes sure the compiler can fold
those conditionals or builtins into .CLZ internal calls with 2 arguments which
makes it well defined for all inputs.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug tree-optimization/115073] RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO
  2024-05-13 15:02 [Bug tree-optimization/115073] New: RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO kito at gcc dot gnu.org
  2024-05-13 15:09 ` [Bug tree-optimization/115073] " jakub at gcc dot gnu.org
@ 2024-05-13 15:14 ` pinskia at gcc dot gnu.org
  2024-05-13 15:25 ` jakub at gcc dot gnu.org
  2024-05-13 15:28 ` pinskia at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-05-13 15:14 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115073

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |INVALID
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Invalid as explained.  The builtin has undefined behavior no matter what the
backend says is the value at 0.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug tree-optimization/115073] RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO
  2024-05-13 15:02 [Bug tree-optimization/115073] New: RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO kito at gcc dot gnu.org
  2024-05-13 15:09 ` [Bug tree-optimization/115073] " jakub at gcc dot gnu.org
  2024-05-13 15:14 ` pinskia at gcc dot gnu.org
@ 2024-05-13 15:25 ` jakub at gcc dot gnu.org
  2024-05-13 15:28 ` pinskia at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: jakub at gcc dot gnu.org @ 2024-05-13 15:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115073

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Or another option is to use C23 <stdbit.h>, that is well defined for all
values.
But you need recent glibc (2.39 snapshots or 2.40) & gcc (14) for that.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug tree-optimization/115073] RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO
  2024-05-13 15:02 [Bug tree-optimization/115073] New: RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO kito at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2024-05-13 15:25 ` jakub at gcc dot gnu.org
@ 2024-05-13 15:28 ` pinskia at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-05-13 15:28 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115073

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note -fsanitize=undefined does detect this undefinedness at runtime already
too.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-05-13 15:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-13 15:02 [Bug tree-optimization/115073] New: RISC-V: Gimple fold not honor C[LT]Z_DEFINED_VALUE_AT_ZERO kito at gcc dot gnu.org
2024-05-13 15:09 ` [Bug tree-optimization/115073] " jakub at gcc dot gnu.org
2024-05-13 15:14 ` pinskia at gcc dot gnu.org
2024-05-13 15:25 ` jakub at gcc dot gnu.org
2024-05-13 15:28 ` pinskia at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).