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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot 
gnu.org

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Drea Pinski from comment #7)
> I think r15-1391-g4b75ed33fa5fd6 caused this.

Well, it exposes that (IIRC) a _Bool load/store does not imply narrowing
to _Bool (it's simply UB), but the cast _is_ explicit narrowing.  What
LIM does is correctly seeing the access as not trapping, but it doesn't
realize that the load is conditional UB.  I'm not sure we formally
define the IL enough here, but there's similar issues with introducing
load-store pairs for x87 FP data - for that we introduced
mode_can_transfer_bits, we cannot do the load of the original data
and speculate the store, instead we'd have to force flag-based SM.
The _Bool case would be similar - we cannot speculate the load/store.

cselim should also be affected btw, just the explicitly emitted cast
is probably what actually breaks it for LIM - and it's
rewrite_to_defined_unconditional which does that, changing the load
to an unsigned char and then emitting a cast.  That seems to be
what triggers the wrong-code for the UB that's introduced by LIM.

The following fixes this, forcing conditional SM for the memory
operations gimple_needing_rewrite_undefined identifies.

diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc
index 72e19981698..4f7401e2d5d 100644
--- a/gcc/tree-ssa-loop-im.cc
+++ b/gcc/tree-ssa-loop-im.cc
@@ -2323,7 +2323,14 @@ execute_sm (class loop *loop, im_mem_ref *ref,
   bool always_stored = ref_always_accessed_p (loop, ref, true);
   if (maybe_mt
       && (bb_in_transaction (loop_preheader_edge (loop)->src)
-         || (ref_can_have_store_data_races (ref->mem.ref) && !
always_stored)))
+         || (ref_can_have_store_data_races (ref->mem.ref) && ! always_stored)
+         /* Do not speculate a load/store when that's not a noop, either
+            because the mode cannot be transferred or because there's
+            UB involved for out-of-bound values.  */
+         || !mode_can_transfer_bits (TYPE_MODE (TREE_TYPE (ref->mem.ref)))
+         || TREE_CODE (TREE_TYPE (ref->mem.ref)) == BOOLEAN_TYPE
+         || (TREE_CODE (ref->mem.ref) == COMPONENT_REF
+             && DECL_BIT_FIELD (TREE_OPERAND (ref->mem.ref, 1)))))
     multi_threaded_model_p = true;

   if (multi_threaded_model_p && !use_other_flag_var)

Reply via email to