[Bug middle-end/94600] Ignored volatile specifier on loop unrolling and bitfield misoptimization

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
We're going through

rtx
store_expr (tree exp, rtx target, int call_param_p,
            bool nontemporal, bool reverse)
{
...
  normal_expr:
      /* If we want to use a nontemporal or a reverse order store, force the
         value into a register first.  */
      tmp_target = nontemporal || reverse ? NULL_RTX : target;
      temp = expand_expr_real (exp, tmp_target, GET_MODE (target),
                               (call_param_p
                                ? EXPAND_STACK_PARM : EXPAND_NORMAL),
                               &alt_rtl, false);

which expands directly into target via

        else if (optimize >= 1
                 && modifier != EXPAND_CONST_ADDRESS
                 && modifier != EXPAND_INITIALIZER
                 && modifier != EXPAND_MEMORY
                 && TREE_READONLY (array) && ! TREE_SIDE_EFFECTS (array)
                 && TREE_CODE (index) == INTEGER_CST
                 && (VAR_P (array) || TREE_CODE (array) == CONST_DECL)
                 && (init = ctor_for_folding (array)) != error_mark_node)
          { 
...
                          /* If VALUE is a CONSTRUCTOR, this
                             optimization is only useful if
                             this doesn't store the CONSTRUCTOR
                             into memory.  If it does, it is more
                             efficient to just load the data from
                             the array directly.  */
                          rtx ret = expand_constructor (value, target,
                                                        modifier, true);

and then store_constructor (exp, target, 0, int_expr_size (exp), false);

store_constructor cannot anymore fail and since we're invoking
expand_constructor with avoid_temp_mem == true that's the place to
fixup I think, possibly by amending

  /* Handle calls that pass values in multiple non-contiguous
     locations.  The Irix 6 ABI has examples of this.  */
  if (target == 0 || ! safe_from_p (target, exp, 1)
      || GET_CODE (target) == PARALLEL || modifier == EXPAND_STACK_PARM)
    { 
      if (avoid_temp_mem)
        return NULL_RTX;

with the case for volatile target.  Or of course at the very toplevel
by passing down a different expand modifier or by skipping ctor folding.

Note that an aggregate volatile assignment is not very well specified
when bitfields are involved - C says the copy is elementwise and then
strict volatile bitfield semantics would suggest that N stores happen
together with the required reads for RMW cycles.

So I'm not sure if this is really a bug or rather wrong user expectation.