Support unaligned load/store on more ARM targets

[Renato Golin]

On 16 May 2013 21:52, JF Bastien <jfb at google.com> wrote:

> I've attached a patch that should address both of your concerns. It adds
> the command-line option, and the defaults stays the same except for ARMv7+
> Linux and PNaCl. I also moved the alignment test away from fast-isel.ll
> into its own file, and added invocations for each of these use cases. That
> allowed me to also rename some of the test functions to be more descriptive.
>
> Please take a look and let me know if this new change does what you would
> expect.
>

The patch looks good to me, except that AllowsUnalignedMem is not
guaranteed to be false by that point on subsequent calls. It was a fault on
the previous code as well, I guess. I wouldn't save the "else false" in the
default case.

Anton, any comments?


I think I understand something: ARM is growing out of its typically
> embedded roots, and I currently care about LLVM's ARM as "consumer ARMv7a",
> whereas you want to also please other users who have typically been closer
> to the system. Would it be worth making a distinction between A/R/M
> profiles at some later time, or something along those lines?
>

It's not a matter of pleasing people, just making sure we don't break
existing code for the sake of localized performance improvements.

And bare metal has nothing to to with the profile of the chip, you can run
bare or OS on any profile A/R/M.


Shouldn't this just be:
>   mrc p15, 0, r0, c1, c0, 0
>   orr r0, r0, #2
>   mcr p15, 0, r0, c1, c0, 0
>

It's not about how hard it is to do it, it's the same as saying that your
code will only run on a machine that has a special feature, when
manufacturers are allowed to not implement them, or OSs not required to set
them.


Agreed that as-is my previous change should, at the minimum, have been
> prominently in release notes, and most probably at some major milestone. I
> hope the attached change is more agreeable.
>

Well, the change as it was couldn't exist, even if they were blinking red
on the release notes. We gave no warning that the default behaviour would
change, and that's just not right. Since we're assuming that Linux will
*always* set unaligned access on v7, that change in default is not as
blatant as bare metal and is probably harmless.

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130516/fba865db/attachment.html>