[Aarch64] Fix conditional branches with target far away.
Sudakshina Das
sudi.das@arm.com
Thu Mar 15 16:56:00 GMT 2018
On 15/03/18 15:27, Sameera Deshpande wrote:
> Ping!
>
> On 28 February 2018 at 16:18, Sameera Deshpande
> <sameera.deshpande@linaro.org> wrote:
>> On 27 February 2018 at 18:25, Ramana Radhakrishnan
>> <ramana.gcc@googlemail.com> wrote:
>>> On Wed, Feb 14, 2018 at 8:30 AM, Sameera Deshpande
>>> <sameera.deshpande@linaro.org> wrote:
>>>> Hi!
>>>>
>>>> Please find attached the patch to fix bug in branches with offsets over 1MiB.
>>>> There has been an attempt to fix this issue in commit
>>>> 050af05b9761f1979f11c151519e7244d5becd7c
>>>>
>>>> However, the far_branch attribute defined in above patch used
>>>> insn_length - which computes incorrect offset. Hence, eliminated the
>>>> attribute completely, and computed the offset from insn_addresses
>>>> instead.
>>>>
>>>> Ok for trunk?
>>>>
>>>> gcc/Changelog
>>>>
>>>> 2018-02-13 Sameera Deshpande <sameera.deshpande@linaro.org>
>>>> * config/aarch64/aarch64.md (far_branch): Remove attribute. Eliminate
>>>> all the dependencies on the attribute from RTL patterns.
>>>>
>>>
>>> I'm not a maintainer but this looks good to me modulo notes about how
>>> this was tested. What would be nice is a testcase for the testsuite as
>>> well as ensuring that the patch has been bootstrapped and regression
>>> tested. AFAIR, the original patch was put in because match.pd failed
>>> when bootstrap in another context.
>>>
>>>
>>> regards
>>> Ramana
>>>
>>>> --
>>>> - Thanks and regards,
>>>> Sameera D.
>>
>> The patch is tested with GCC testsuite and bootstrapping successfully.
>> Also tested for spec benchmark.
>>
I am not a maintainer either. I noticed that the range check you do for
the offset has a (<= || >=). The "far_branch" however did (< || >=) for
a positive value. Was that also part of the incorrect offset calculation?
@@ -692,7 +675,11 @@
{
if (get_attr_length (insn) =3D=3D 8)
{
- if (get_attr_far_branch (insn) =3D=3D 1)
+ long long int offset;
+ offset =3D INSN_ADDRESSES (INSN_UID (XEXP (operands[2], 0)))
+ - INSN_ADDRESSES (INSN_UID (insn));
+
+ if (offset <=3D -1048576 || offset >=3D 1048572)
return aarch64_gen_far_branch (operands, 2, "Ltb",
"<inv_tb>\\t%<w>0, %1, ");
else
@@ -709,12 +696,7 @@
(if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int
-32768))
(lt (minus (match_dup 2) (pc)) (const_int
32764)))
(const_int 4)
- (const_int 8)))
- (set (attr "far_branch")
- (if_then_else (and (ge (minus (match_dup 2) (pc)) (const_int
-1048576))
- (lt (minus (match_dup 2) (pc)) (const_int
1048572)))
- (const_int 0)
- (const_int 1)))]
+ (const_int 8)))]
)
Thanks
Sudi
>> --
>> - Thanks and regards,
>> Sameera D.
>
>
>
More information about the Gcc-patches
mailing list