xiezhiheng <xiezhih...@huawei.com> writes:
>> -----Original Message-----
>> From: Richard Sandiford [mailto:richard.sandif...@arm.com]
>> Sent: Friday, July 17, 2020 5:04 PM
>> To: xiezhiheng <xiezhih...@huawei.com>
>> Cc: Richard Biener <richard.guent...@gmail.com>; gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH PR94442] [AArch64] Redundant ldp/stp instructions
>> emitted at -O3
>>
>
> Cut...
>
>>
>> Thanks, pushed to master.
>>
>> Richard
>
> And I have finished the second part.
>
> In function aarch64_general_add_builtin, I add an argument ATTRS to
> pass attributes for each built-in function.
>
> And some new functions are added:
> aarch64_call_properties: return flags for each built-in function based
> on command-line options. When the built-in function handles
> floating-points, add FLAG_FP flag.
>
> aarch64_modifies_global_state_p: True if the function would modify
> global states.
>
> aarch64_reads_global_state_p: True if the function would read
> global states.
>
> aarch64_could_trap_p: True if the function would raise a signal.
>
> aarch64_add_attribute: Add attributes in ATTRS.
>
> aarch64_get_attributes: return attributes for each built-in functons
> based on flags and command-line options.
>
> In function aarch64_init_simd_builtins, attributes are get by flags
> and pass them to function aarch64_general_add_builtin.
>
>
> Bootstrap is tested OK on aarch64 Linux platform, but regression
> FAIL one test case ---- pr93423.f90.
> However, I found that this test case would fail randomly in trunk.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93423
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96041
> Some PRs have tracked it. After my patch, this test case would
> always fail. I guess the syntax errors in fortran crash some structures
> result in illegal memory access but I can't find what exactly it is.
> But I think my patch should have no influence on it.
Yeah, I agree. And FWIW, I didn't see this in my testing.
I've pushed the patch with one trivial change: to remove the “and”
before “CODE” in:
> /* Wrapper around add_builtin_function. NAME is the name of the built-in
> function, TYPE is the function type, and CODE is the function subcode
> - (relative to AARCH64_BUILTIN_GENERAL). */
> + (relative to AARCH64_BUILTIN_GENERAL), and ATTRS is the function
> + attributes. */
BTW, one thing to be careful of in future is that not all FP intrinsics
raise FP exceptions. So while:
> + switch (d->mode)
> + {
> + /* Floating-point. */
> + case E_BFmode:
> + case E_V4BFmode:
> + case E_V8BFmode:
> + case E_HFmode:
> + case E_V4HFmode:
> + case E_V8HFmode:
> + case E_SFmode:
> + case E_V2SFmode:
> + case E_V4SFmode:
> + case E_DFmode:
> + case E_V1DFmode:
> + case E_V2DFmode:
> + flags |= FLAG_FP;
> + break;
> +
> + default:
> + break;
> + }
is a good, conservatively-correct default, we might need an additional
flag to suppress it for certain intrinsics.
I've just realised that the code above could have used FLOAT_MODE_P,
but I didn't think of that before pushing the patch :-)
Thanks,
Richard
