Re: [PATCH][AARCH64][PR target/84882] Add mno-strict-align

[Sudakshina Das]

Hi Richard

On 18/05/18 15:48, Richard Earnshaw (lists) wrote:

On 27/03/18 13:58, Sudakshina Das wrote:

Hi

This patch adds the no variant to -mstrict-align and the corresponding
function attribute. To enable the function attribute, I have modified
aarch64_can_inline_p () to allow checks even when the callee function
has no attribute. The need for this is shown by the new test
target_attr_18.c.

Testing: Bootstrapped, regtested and added new tests that are copies
of earlier tests checking -mstrict-align with opposite scan directives.

Is this ok for trunk?

Sudi


*** gcc/ChangeLog ***

2018-03-27  Sudakshina Das  

 * common/config/aarch64/aarch64-common.c (aarch64_handle_option):
 Check val before adding MASK_STRICT_ALIGN to opts->x_target_flags.
 * config/aarch64/aarch64.opt (mstrict-align): Remove RejectNegative.
 * config/aarch64/aarch64.c (aarch64_attributes): Mark allow_neg
 as true for strict-align.
 (aarch64_can_inline_p): Perform checks even when callee has no
 attributes to check for strict alignment.
 * doc/extend.texi (AArch64 Function Attributes): Document
 no-strict-align.
 * doc/invoke.texi: (AArch64 Options): Likewise.

*** gcc/testsuite/ChangeLog ***

2018-03-27  Sudakshina Das  

 * gcc.target/aarch64/pr84882.c: New test.
 * gcc.target/aarch64/target_attr_18.c: Likewise.

strict-align.diff


diff --git a/gcc/common/config/aarch64/aarch64-common.c 
b/gcc/common/config/aarch64/aarch64-common.c
index 7fd9305..d5655a0 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -97,7 +97,10 @@ aarch64_handle_option (struct gcc_options *opts,
return true;
  
  case OPT_mstrict_align:

-  opts->x_target_flags |= MASK_STRICT_ALIGN;
+  if (val)
+   opts->x_target_flags |= MASK_STRICT_ALIGN;
+  else
+   opts->x_target_flags &= ~MASK_STRICT_ALIGN;
return true;
  
  case OPT_momit_leaf_frame_pointer:

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 4b5183b..4f35a6c 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11277,7 +11277,7 @@ static const struct aarch64_attribute_info 
aarch64_attributes[] =
{ "fix-cortex-a53-843419", aarch64_attr_bool, true, NULL,
   OPT_mfix_cortex_a53_843419 },
{ "cmodel", aarch64_attr_enum, false, NULL, OPT_mcmodel_ },
-  { "strict-align", aarch64_attr_mask, false, NULL, OPT_mstrict_align },
+  { "strict-align", aarch64_attr_mask, true, NULL, OPT_mstrict_align },
{ "omit-leaf-frame-pointer", aarch64_attr_bool, true, NULL,
   OPT_momit_leaf_frame_pointer },
{ "tls-dialect", aarch64_attr_enum, false, NULL, OPT_mtls_dialect_ },
@@ -11640,16 +11640,13 @@ aarch64_can_inline_p (tree caller, tree callee)
tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);
tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee);
  
-  /* If callee has no option attributes, then it is ok to inline.  */

-  if (!callee_tree)
-return true;


I think it's still useful to spot the case where both callee_tree and
caller_tree are NULL.  In that case both options will pick up
target_option_default_node and will always be compatible; so you can
short-circuit that case, which is the most likely scenario.


-
struct cl_target_option *caller_opts
= TREE_TARGET_OPTION (caller_tree ? caller_tree
   : target_option_default_node);
  
-  struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree);

-
+  struct cl_target_option *callee_opts
+   = TREE_TARGET_OPTION (callee_tree ? callee_tree
+  : target_option_default_node);
  
/* Callee's ISA flags should be a subset of the caller's.  */

if ((caller_opts->x_aarch64_isa_flags & callee_opts->x_aarch64_isa_flags)
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 52eaf8c..1426b45 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -85,7 +85,7 @@ Target RejectNegative Joined Enum(cmodel) 
Var(aarch64_cmodel_var) Init(AARCH64_C
  Specify the code model.
  
  mstrict-align

-Target Report RejectNegative Mask(STRICT_ALIGN) Save
+Target Report Mask(STRICT_ALIGN) Save
  Don't assume that unaligned accesses are handled by the system.
  
  momit-leaf-frame-pointer

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 93a0ebc..dcda216 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3605,8 +3605,10 @@ for the command line option @option{-mcmodel=}.
  @item strict-align


Other targets add an @itemx for the no-variant.


  @cindex @code{strict-align} function attribute, AArch64
  Indicates that the compiler should not assume that unaligned memory references
-are handled by the system.  The behavior is the same as for the command-line
-option @option{-mstrict-align}.
+are handled by the system.  

Re: [PATCH][AARCH64][PR target/84882] Add mno-strict-align

[Richard Earnshaw (lists)]

On 27/03/18 13:58, Sudakshina Das wrote:
> Hi
> 
> This patch adds the no variant to -mstrict-align and the corresponding
> function attribute. To enable the function attribute, I have modified
> aarch64_can_inline_p () to allow checks even when the callee function
> has no attribute. The need for this is shown by the new test
> target_attr_18.c.
> 
> Testing: Bootstrapped, regtested and added new tests that are copies
> of earlier tests checking -mstrict-align with opposite scan directives.
> 
> Is this ok for trunk?
> 
> Sudi
> 
> 
> *** gcc/ChangeLog ***
> 
> 2018-03-27  Sudakshina Das  
> 
> * common/config/aarch64/aarch64-common.c (aarch64_handle_option):
> Check val before adding MASK_STRICT_ALIGN to opts->x_target_flags.
> * config/aarch64/aarch64.opt (mstrict-align): Remove RejectNegative.
> * config/aarch64/aarch64.c (aarch64_attributes): Mark allow_neg
> as true for strict-align.
> (aarch64_can_inline_p): Perform checks even when callee has no
> attributes to check for strict alignment.
> * doc/extend.texi (AArch64 Function Attributes): Document
> no-strict-align.
> * doc/invoke.texi: (AArch64 Options): Likewise.
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2018-03-27  Sudakshina Das  
> 
> * gcc.target/aarch64/pr84882.c: New test.
> * gcc.target/aarch64/target_attr_18.c: Likewise.
> 
> strict-align.diff
> 
> 
> diff --git a/gcc/common/config/aarch64/aarch64-common.c 
> b/gcc/common/config/aarch64/aarch64-common.c
> index 7fd9305..d5655a0 100644
> --- a/gcc/common/config/aarch64/aarch64-common.c
> +++ b/gcc/common/config/aarch64/aarch64-common.c
> @@ -97,7 +97,10 @@ aarch64_handle_option (struct gcc_options *opts,
>return true;
>  
>  case OPT_mstrict_align:
> -  opts->x_target_flags |= MASK_STRICT_ALIGN;
> +  if (val)
> + opts->x_target_flags |= MASK_STRICT_ALIGN;
> +  else
> + opts->x_target_flags &= ~MASK_STRICT_ALIGN;
>return true;
>  
>  case OPT_momit_leaf_frame_pointer:
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 4b5183b..4f35a6c 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -11277,7 +11277,7 @@ static const struct aarch64_attribute_info 
> aarch64_attributes[] =
>{ "fix-cortex-a53-843419", aarch64_attr_bool, true, NULL,
>   OPT_mfix_cortex_a53_843419 },
>{ "cmodel", aarch64_attr_enum, false, NULL, OPT_mcmodel_ },
> -  { "strict-align", aarch64_attr_mask, false, NULL, OPT_mstrict_align },
> +  { "strict-align", aarch64_attr_mask, true, NULL, OPT_mstrict_align },
>{ "omit-leaf-frame-pointer", aarch64_attr_bool, true, NULL,
>   OPT_momit_leaf_frame_pointer },
>{ "tls-dialect", aarch64_attr_enum, false, NULL, OPT_mtls_dialect_ },
> @@ -11640,16 +11640,13 @@ aarch64_can_inline_p (tree caller, tree callee)
>tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);
>tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee);
>  
> -  /* If callee has no option attributes, then it is ok to inline.  */
> -  if (!callee_tree)
> -return true;

I think it's still useful to spot the case where both callee_tree and
caller_tree are NULL.  In that case both options will pick up
target_option_default_node and will always be compatible; so you can
short-circuit that case, which is the most likely scenario.

> -
>struct cl_target_option *caller_opts
>   = TREE_TARGET_OPTION (caller_tree ? caller_tree
>  : target_option_default_node);
>  
> -  struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree);
> -
> +  struct cl_target_option *callee_opts
> + = TREE_TARGET_OPTION (callee_tree ? callee_tree
> +: target_option_default_node);
>  
>/* Callee's ISA flags should be a subset of the caller's.  */
>if ((caller_opts->x_aarch64_isa_flags & callee_opts->x_aarch64_isa_flags)
> diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
> index 52eaf8c..1426b45 100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -85,7 +85,7 @@ Target RejectNegative Joined Enum(cmodel) 
> Var(aarch64_cmodel_var) Init(AARCH64_C
>  Specify the code model.
>  
>  mstrict-align
> -Target Report RejectNegative Mask(STRICT_ALIGN) Save
> +Target Report Mask(STRICT_ALIGN) Save
>  Don't assume that unaligned accesses are handled by the system.
>  
>  momit-leaf-frame-pointer
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 93a0ebc..dcda216 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -3605,8 +3605,10 @@ for the command line option @option{-mcmodel=}.
>  @item strict-align

Other targets add an @itemx for the no-variant.

>  @cindex @code{strict-align} function attribute, AArch64
>  Indicates that the compiler should not assume that unaligned memory 
> references
> -are handled by the system.  The 

Re: [PATCH][AARCH64][PR target/84882] Add mno-strict-align

[Kyrill Tkachov]

Hi Sudi,

On 27/03/18 13:58, Sudakshina Das wrote:

Hi

This patch adds the no variant to -mstrict-align and the corresponding
function attribute. To enable the function attribute, I have modified
aarch64_can_inline_p () to allow checks even when the callee function
has no attribute. The need for this is shown by the new test
target_attr_18.c.

Testing: Bootstrapped, regtested and added new tests that are copies
of earlier tests checking -mstrict-align with opposite scan directives.

Is this ok for trunk?



This looks ok to me but you'll need approval from a maintainer.
Please put the PR marker in your ChangeLog so that the svn hook picks it up on 
commit.

Thanks,
Kyrill


Sudi


*** gcc/ChangeLog ***

2018-03-27  Sudakshina Das  

* common/config/aarch64/aarch64-common.c (aarch64_handle_option):
Check val before adding MASK_STRICT_ALIGN to opts->x_target_flags.
* config/aarch64/aarch64.opt (mstrict-align): Remove RejectNegative.
* config/aarch64/aarch64.c (aarch64_attributes): Mark allow_neg
as true for strict-align.
(aarch64_can_inline_p): Perform checks even when callee has no
attributes to check for strict alignment.
* doc/extend.texi (AArch64 Function Attributes): Document
no-strict-align.
* doc/invoke.texi: (AArch64 Options): Likewise.

*** gcc/testsuite/ChangeLog ***

2018-03-27  Sudakshina Das  

* gcc.target/aarch64/pr84882.c: New test.
* gcc.target/aarch64/target_attr_18.c: Likewise.

Re: [PATCH][AARCH64][PR target/84882] Add mno-strict-align

[Sudakshina Das]

Ping!

On 27/03/18 13:58, Sudakshina Das wrote:

Hi

This patch adds the no variant to -mstrict-align and the corresponding
function attribute. To enable the function attribute, I have modified
aarch64_can_inline_p () to allow checks even when the callee function
has no attribute. The need for this is shown by the new test
target_attr_18.c.

Testing: Bootstrapped, regtested and added new tests that are copies
of earlier tests checking -mstrict-align with opposite scan directives.

Is this ok for trunk?

Sudi


*** gcc/ChangeLog ***

2018-03-27  Sudakshina Das  

 * common/config/aarch64/aarch64-common.c (aarch64_handle_option):
 Check val before adding MASK_STRICT_ALIGN to opts->x_target_flags.
 * config/aarch64/aarch64.opt (mstrict-align): Remove RejectNegative.
 * config/aarch64/aarch64.c (aarch64_attributes): Mark allow_neg
 as true for strict-align.
 (aarch64_can_inline_p): Perform checks even when callee has no
 attributes to check for strict alignment.
 * doc/extend.texi (AArch64 Function Attributes): Document
 no-strict-align.
 * doc/invoke.texi: (AArch64 Options): Likewise.

*** gcc/testsuite/ChangeLog ***

2018-03-27  Sudakshina Das  

 * gcc.target/aarch64/pr84882.c: New test.
 * gcc.target/aarch64/target_attr_18.c: Likewise.

[PATCH][AARCH64][PR target/84882] Add mno-strict-align

[Sudakshina Das]

Hi

This patch adds the no variant to -mstrict-align and the corresponding
function attribute. To enable the function attribute, I have modified
aarch64_can_inline_p () to allow checks even when the callee function
has no attribute. The need for this is shown by the new test
target_attr_18.c.

Testing: Bootstrapped, regtested and added new tests that are copies
of earlier tests checking -mstrict-align with opposite scan directives.

Is this ok for trunk?

Sudi


*** gcc/ChangeLog ***

2018-03-27  Sudakshina Das  

* common/config/aarch64/aarch64-common.c (aarch64_handle_option):
Check val before adding MASK_STRICT_ALIGN to opts->x_target_flags.
* config/aarch64/aarch64.opt (mstrict-align): Remove RejectNegative.
* config/aarch64/aarch64.c (aarch64_attributes): Mark allow_neg
as true for strict-align.
(aarch64_can_inline_p): Perform checks even when callee has no
attributes to check for strict alignment.
* doc/extend.texi (AArch64 Function Attributes): Document
no-strict-align.
* doc/invoke.texi: (AArch64 Options): Likewise.

*** gcc/testsuite/ChangeLog ***

2018-03-27  Sudakshina Das  

* gcc.target/aarch64/pr84882.c: New test.
* gcc.target/aarch64/target_attr_18.c: Likewise.
diff --git a/gcc/common/config/aarch64/aarch64-common.c b/gcc/common/config/aarch64/aarch64-common.c
index 7fd9305..d5655a0 100644
--- a/gcc/common/config/aarch64/aarch64-common.c
+++ b/gcc/common/config/aarch64/aarch64-common.c
@@ -97,7 +97,10 @@ aarch64_handle_option (struct gcc_options *opts,
   return true;
 
 case OPT_mstrict_align:
-  opts->x_target_flags |= MASK_STRICT_ALIGN;
+  if (val)
+	opts->x_target_flags |= MASK_STRICT_ALIGN;
+  else
+	opts->x_target_flags &= ~MASK_STRICT_ALIGN;
   return true;
 
 case OPT_momit_leaf_frame_pointer:
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 4b5183b..4f35a6c 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11277,7 +11277,7 @@ static const struct aarch64_attribute_info aarch64_attributes[] =
   { "fix-cortex-a53-843419", aarch64_attr_bool, true, NULL,
  OPT_mfix_cortex_a53_843419 },
   { "cmodel", aarch64_attr_enum, false, NULL, OPT_mcmodel_ },
-  { "strict-align", aarch64_attr_mask, false, NULL, OPT_mstrict_align },
+  { "strict-align", aarch64_attr_mask, true, NULL, OPT_mstrict_align },
   { "omit-leaf-frame-pointer", aarch64_attr_bool, true, NULL,
  OPT_momit_leaf_frame_pointer },
   { "tls-dialect", aarch64_attr_enum, false, NULL, OPT_mtls_dialect_ },
@@ -11640,16 +11640,13 @@ aarch64_can_inline_p (tree caller, tree callee)
   tree caller_tree = DECL_FUNCTION_SPECIFIC_TARGET (caller);
   tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (callee);
 
-  /* If callee has no option attributes, then it is ok to inline.  */
-  if (!callee_tree)
-return true;
-
   struct cl_target_option *caller_opts
 	= TREE_TARGET_OPTION (caller_tree ? caller_tree
 	   : target_option_default_node);
 
-  struct cl_target_option *callee_opts = TREE_TARGET_OPTION (callee_tree);
-
+  struct cl_target_option *callee_opts
+	= TREE_TARGET_OPTION (callee_tree ? callee_tree
+	   : target_option_default_node);
 
   /* Callee's ISA flags should be a subset of the caller's.  */
   if ((caller_opts->x_aarch64_isa_flags & callee_opts->x_aarch64_isa_flags)
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 52eaf8c..1426b45 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -85,7 +85,7 @@ Target RejectNegative Joined Enum(cmodel) Var(aarch64_cmodel_var) Init(AARCH64_C
 Specify the code model.
 
 mstrict-align
-Target Report RejectNegative Mask(STRICT_ALIGN) Save
+Target Report Mask(STRICT_ALIGN) Save
 Don't assume that unaligned accesses are handled by the system.
 
 momit-leaf-frame-pointer
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 93a0ebc..dcda216 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3605,8 +3605,10 @@ for the command line option @option{-mcmodel=}.
 @item strict-align
 @cindex @code{strict-align} function attribute, AArch64
 Indicates that the compiler should not assume that unaligned memory references
-are handled by the system.  The behavior is the same as for the command-line
-option @option{-mstrict-align}.
+are handled by the system.  To allow the compiler to assume that aligned memory
+references are handled by the system, the inverse attribute
+@code{no-strict-align} can be specified.  The behavior is the same as for the
+command-line option @option{-mstrict-align} and @option{-mno-strict-align}.
 
 @item omit-leaf-frame-pointer
 @cindex @code{omit-leaf-frame-pointer} function attribute, AArch64
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index feacd56..0574d21 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -596,7 +596,7 @@ Objective-C