This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
- From: Ramana Radhakrishnan <ramana dot gcc at googlemail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: Charles Baylis <charles dot baylis at linaro dot org>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>, Joey Ye <joey dot ye at arm dot com>
- Date: Fri, 4 Apr 2014 15:50:28 +0100
- Subject: Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)
- Authentication-results: sourceware.org; auth=none
- References: <CADnVucBLd0YBEWsJGFVNTktuGh-72cxXmNH0HnK-+6TytO-s6Q at mail dot gmail dot com> <CAJA7tRYCROs+SqrAMZ4weBz+ZxqVQMAY-4zOuSr99ANhFLB8DA at mail dot gmail dot com> <533EB72C dot 40109 at redhat dot com>
- Reply-to: ramrad01 at arm dot com
>>
>> My first reaction is to wonder why this is this not a bug in the
>> "shorten" phase.
>
> I don't think that code ever expected an alignment directive to be emitted
> by ASM_OUTPUT_CASE_END :(
Fair point and it looks like this support came in when the support for
Thumb2 was added eons ago.
>
>
>
>>
>>>
>>> This patch addresses the issue by removing ASM_OUTPUT_CASE_END from
>>> arm.h, and ensuring that the alignment after an ADDR_DIFF_VEC is
>>> instead inserted by aligning the label following the barrier which
>>> follows it. This is achieved by defining LABEL_ALIGN_AFTER_BARRIER
>>> appropriately.
>>
>>
>> On first glance this feels like a blunt hammer, what's the code size
>> bloat with putting out such an alignment after each barrier that the
>> compiler emits rather than tracking this in ASM_OUTPUT_CASE_END.
>
> I'd tend to agree that emitting an alignment after each barrier would be a
> blunt hammer in this case.
>
> ISTM we really want a new target hook to define the alignment after a the
> jump table, independent of the other alignment directives. Then we'd have
> to teach shorten_branches about that.
>
> Perhaps the blunt hammer for 4.9 and the new alignemnt-after-jump-table for
> the next stage1?
Given the following below - it may not be that blunt at all. I don't
know how I missed this yesterday (thanks to Charlie for pointing this
out on IRC) :(
+#define LABEL_ALIGN_AFTER_BARRIER(LABEL) \
+ (GET_CODE (PATTERN (prev_active_insn (LABEL))) == ADDR_DIFF_VEC \
+ ? 1 : 0)
I think we should keep this on trunk for a week to check if there is
no unintended fallout before backporting to 4.8
Additionally the testing has only considered Thumb2 - since we also do
jumptable shortening for Thumb1 and given this late change it's worth
also testing this on Thumb1 and making sure there are no regressions.
Maybe Joey can help there if you aren't set up to do this.
Ok if no regressions and modulo RM objections.
One minor Changelog nit.
2014-04-02 Charles Baylis <charles.baylis@linaro.org>
PR target/60609
* config/arm/arm.h (ASM_OUTPUT_CASE_END) Remove.
(LABEL_ALIGN_AFTER_BARRIER) Align barriers which occur after
ADDR_DIFF_VEC.
s/)/):/g above.
regards
Ramana
>
> jeff
>