[Bug target/71106] MIPS: Unaligned load/store instructions are not used to access a variable with an aligned attribute

[pinskia at gcc dot gnu.org via Gcc-bugs]

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

Andrew Pinski  changed:

   What|Removed |Added

 Resolution|--- |DUPLICATE
 Status|NEW |RESOLVED

--- Comment #4 from Andrew Pinski  ---
this is actually a dup of bug 88085.  The problem is aligned attribute is
(incorrectly) ignored partly when used on variable definitions/declarations.

*** This bug has been marked as a duplicate of bug 88085 ***

[Bug target/71106] MIPS: Unaligned load/store instructions are not used to access a variable with an aligned attribute

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski  ---
Doing a movmisalign for mips is actually very easy:


;; Unaligned load/store (movmisalign)
(define_expand "movmisalign"
  [(set (match_operand:GPR 0 "nonimmediate_operand")
(match_operand:GPR 1 "move_operand"))]
 ""
{
  if (MEM_P (operands[0]))
{
  operands[1] = force_reg (mode, operands[1]);
  if (!mips_expand_ins_as_unaligned_store (operands[0],
   operands[1],
   GET_MODE_BITSIZE (mode),
0))
gcc_unreachable ();
}
  else if (MEM_P (operands[1]))
   {
  if (!mips_expand_ext_as_unaligned_load (operands[0],
  operands[1],
  GET_MODE_BITSIZE (mode), 0,
0))
gcc_unreachable ();
}
  else
gcc_unreachable ();
  DONE;
})

 CUT -
I had this in Cavium's GCC for years now but I never got around to testing it
to the trunk.

There was one issue with it, the MEM did not always have an alignment set on
it.
Sprinkling some "set_mem_align (mem, align);" in expr.c fixes that issue.
--- CUT ---
The testcase which fails was:
extern int nc;
void f(void)
{
unsigned char resp[1024];
int c;
int bl = 0;
unsigned long long *dwords = (unsigned long long *)(resp + 5);
for (c=0; c

[Bug target/71106] MIPS: Unaligned load/store instructions are not used to access a variable with an aligned attribute

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2016-05-17
 Ever confirmed|0   |1

--- Comment #2 from Eric Botcazou  ---
> Index: gcc/expr.c
> ===
> --- gcc/expr.c  (revision 236309)
> +++ gcc/expr.c  (working copy)
> @@ -4654,9 +4654,7 @@ expand_assignment (tree to, tree from, b
>  
>/* Handle misaligned stores.  */
>mode = TYPE_MODE (TREE_TYPE (to));
> -  if ((TREE_CODE (to) == MEM_REF
> -   || TREE_CODE (to) == TARGET_MEM_REF)
> -  && mode != BLKmode
> +  if (mode != BLKmode
>&& !mem_ref_refers_to_non_mem_p (to)
>&& ((align = get_object_alignment (to))
>   < GET_MODE_ALIGNMENT (mode))
> 
> would fix that.  Without pruning some of the "pessimistic" handling in
> get_object_alignment this will likely result in some code-gen regressions
> though.
> 
> Index: gcc/builtins.c
> ===
> --- gcc/builtins.c  (revision 236309)
> +++ gcc/builtins.c  (working copy)
> @@ -339,7 +339,7 @@ get_object_alignment_2 (tree exp, unsign
>  Do so only if get_pointer_alignment_1 did not reveal absolute
>  alignment knowledge and if using that alignment would
>  improve the situation.  */
> -  if (!addr_p && !known_alignment
> +  if (!addr_p
>   && TYPE_ALIGN (TREE_TYPE (exp)) > align)
> align = TYPE_ALIGN (TREE_TYPE (exp));
>else

The above change only affects indirect references as bases though, so I'm not
sure whether it will do anything here.  The expander change looks OK to me if
we want to support this kind of nonsense on strict-alignment platforms.

[Bug target/71106] MIPS: Unaligned load/store instructions are not used to access a variable with an aligned attribute

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 Target||mips
 CC||ebotcazou at gcc dot gnu.org

--- Comment #1 from Richard Biener  ---
I think that is "expected" behavior for strict-aling targets that do not
provide
a movmisalign handler.  But newer GCC (like GCC 6 you used) should behave
correctly here.  The issue seems to be that the misalign handling is restricted
to indirect references in expand_assignment.

Thus

Index: gcc/expr.c
===
--- gcc/expr.c  (revision 236309)
+++ gcc/expr.c  (working copy)
@@ -4654,9 +4654,7 @@ expand_assignment (tree to, tree from, b

   /* Handle misaligned stores.  */
   mode = TYPE_MODE (TREE_TYPE (to));
-  if ((TREE_CODE (to) == MEM_REF
-   || TREE_CODE (to) == TARGET_MEM_REF)
-  && mode != BLKmode
+  if (mode != BLKmode
   && !mem_ref_refers_to_non_mem_p (to)
   && ((align = get_object_alignment (to))
  < GET_MODE_ALIGNMENT (mode))

would fix that.  Without pruning some of the "pessimistic" handling in
get_object_alignment this will likely result in some code-gen regressions
though.

Index: gcc/builtins.c
===
--- gcc/builtins.c  (revision 236309)
+++ gcc/builtins.c  (working copy)
@@ -339,7 +339,7 @@ get_object_alignment_2 (tree exp, unsign
 Do so only if get_pointer_alignment_1 did not reveal absolute
 alignment knowledge and if using that alignment would
 improve the situation.  */
-  if (!addr_p && !known_alignment
+  if (!addr_p
  && TYPE_ALIGN (TREE_TYPE (exp)) > align)
align = TYPE_ALIGN (TREE_TYPE (exp));
   else