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

Sudakshina Das sudi.das@arm.com
Wed May 23 11:47:00 GMT 2018 

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  <sudi.das@arm.com>
>>
>>      * 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  <sudi.das@arm.com>
>>
>>      * 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.  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}.
> 
> You've two options now, so 'options'.
> 
>>   
>>   @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 and Objective-C++ Dialects}.
>>   @gccoptlist{-mabi=@var{name}  -mbig-endian  -mlittle-endian @gol
>>   -mgeneral-regs-only @gol
>>   -mcmodel=tiny  -mcmodel=small  -mcmodel=large @gol
>> --mstrict-align @gol
>> +-mstrict-align -mno-strict-align @gol
>>   -momit-leaf-frame-pointer @gol
>>   -mtls-dialect=desc  -mtls-dialect=traditional @gol
>>   -mtls-size=@var{size} @gol
>> @@ -14601,9 +14601,11 @@ Generate code for the large code model.  This makes no assumptions about
>>   addresses and sizes of sections.  Programs can be statically linked only.
>>   
>>   @item -mstrict-align
>> +@itemx -mno-strict-align
>>   @opindex mstrict-align
>> -Avoid generating memory accesses that may not be aligned on a natural object
>> -boundary as described in the architecture specification.
>> +@opindex mno-strict-align
>> +Avoid or allow generating memory accesses that may not be aligned on a natural
>> +object boundary as described in the architecture specification.
>>   
>>   @item -momit-leaf-frame-pointer
>>   @itemx -mno-omit-leaf-frame-pointer
>> diff --git a/gcc/testsuite/gcc.target/aarch64/pr84882.c b/gcc/testsuite/gcc.target/aarch64/pr84882.c
>> new file mode 100644
>> index 0000000..ffc7d55
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/pr84882.c
>> @@ -0,0 +1,33 @@
>> +/* This is a copy of pr71727.c with scanning reversed.  */
>> +/* { dg-do compile } */
>> +/* { dg-options "-mstrict-align -O3 -mno-strict-align" } */
>> +
>> +struct test_struct_s
>> +{
>> +  long a;
>> +  long b;
>> +  long c;
>> +  long d;
>> +  unsigned long e;
>> +};
>> +
>> +
>> +char _a;
>> +struct test_struct_s xarray[128];
>> +
>> +void
>> +_start (void)
>> +{
>> +  struct test_struct_s *new_entry;
>> +
>> +  new_entry = &xarray[0];
>> +  new_entry->a = 1;
>> +  new_entry->b = 2;
>> +  new_entry->c = 3;
>> +  new_entry->d = 4;
>> +  new_entry->e = 5;
>> +
>> +  return;
>> +}
>> +
>> +/* { dg-final { scan-assembler "add\tx[0-9]+, x[0-9]+, :" {target lp64} } } */
>> diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_18.c b/gcc/testsuite/gcc.target/aarch64/target_attr_18.c
>> new file mode 100644
>> index 0000000..33b1d44
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_18.c
>> @@ -0,0 +1,21 @@
>> +/* This is a copy of target_attr_6.c to instead check no-strict-align.  */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O2 -save-temps -mstrict-align" } */
>> +
>> +/* Inlining strict-align functions into non-strict align
>> +   functions is not allowed.  */
>> +
>> +int
>> +bar (int a)
>> +{
>> +  return a - 6;
>> +}
>> +
>> +__attribute__ ((target ("no-strict-align")))
>> +int
>> +bam (int a)
>> +{
>> +  return a - bar (a);
>> +}
>> +
>> +/* { dg-final { scan-assembler "bl.*bar" } } */
>>
> 
> OK with those changes.  As Kyrill said, don't forget the PR number in
> the changelog.
> 

Thank you for approving. Committed with suggested changes as r260604.

Sudi
> R.
>

More information about the Gcc-patches mailing list