This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, ARM] Fix PR60609 (Error: value of 256 too large for field of 1 bytes)


>>
>> 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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]