[PATCH] D11075: LLVM gen correct asm info for mingw and cygwin arm targets

[Martell Malone]

martell added a comment.

In http://reviews.llvm.org/D11075#202824, @t.p.northover wrote:

> I'm afraid I don't think these are the right tests to be adding.
>
> They all seem to pass already, so they're unrelated to the change you're making. The only difference I've seen during some brief fiddling is when -filetype=obj is used (your change prevents some assertion failure), though looking at the source it probably affects some other details too.


Yes you are correct it does prevent an assertion failure.
Itanium seems to use these tests for that purpose also so I don't think I should make a specific assertion test for gnu when there isn't one for itanium or msvc.
The test structors.ll has a different section name with gnu so this test would most definitely fail if the target wasn't supported.

The only other test I can think of todo is alloca.ll where we use __chkstk_ms instead of __chkstk but I will put that in a separate commit as i have to edit the code gen for that.

In http://reviews.llvm.org/D11075#202858, @rnk wrote:

> OK, so mingw will presumably also be focusing on a thumb-only, winrt, environment?


Yes the target will windows NT just like msvc and itanium :)
Windows store / Desktop

> Can you add a comment about the purpose of this test? Is the code sequence below actually a PIC sequence?


I could be wrong but to me the purpose of this test is to ensure to load of an external global is done via the movw movt pair
A branch is done the same way and this test seems to ensure the relocation model doesn't affect this.
I was actually not expecting gnu to do this the correct way.

> It's probably cleaner to flip the order to this:

> 

>   else if (TheTriple.isWindowsMSVCEnvironment())

>    MAI = new ARMCOFFMCAsmInfoMicrosoft();

>   else if (TheTriple.isOSWindows())

>    MAI = new ARMCOFFMCAsmInfoGNU();

>   else ...

> 

> We basically have the MS environment and then the GNU-ish "everything else" one.


I will update the patch to reflect this :)


http://reviews.llvm.org/D11075