[PATCH] D22776: [ARM] Set the thumb flag for all text segments on COFF (partial revert of recent commit)

[Martin Storsjö via llvm-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D22776#497392, @rengolin wrote:

> The **only** "quick fix" that I'd find acceptable here would be something along the lines of:
>
>   bool isWinARM = (T.getOS() == Triple::Windows &&
>                   (T.getArch() == Triple::arm || T.getArch() == Triple::thumb));
>   
>
> then replace "isThumb" by "isWinARM" and add a big FIXME saying Windows on ARM Triples are broken.


For correctness, shouldn't you keep setting the flag also for non-windows thumb? I.e. something like this:

  bool isWinARM = (T.getOS() == Triple::Windows &&
                  (T.getArch() == Triple::arm || T.getArch() == Triple::thumb));
  bool isThumb = T.getArch() == Triple::thumb;
  [...]
      ((isThumb || isWinARM) ? COFF::IMAGE_SCN_MEM_16BIT : (COFF::SectionCharacteristics)0)

> But I'd be **much** happier with a proper fix to the Triple on ARM/Windows.


The proper fix isn't so much about triples I think, as about tracking the thumb/arm mode flag and propagating it here. Those flags are per-function - what if there are mixed thumb/arm functions in a single text section (when this flag here is set per section)? Ignoring the contrieved case of mixed mode functions, you'd still need to look up e.g. the first function in the segment, and I'm not sure if that's available at all at this point.

As for why the triples are wrong, see lines 487-503 at http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?revision=276848&view=markup - this is not about windows per se, but the clang driver always passes an arm triple to the assembler, regardless of what is set on the command line (even for all other OSes). The issue is that COFF needs to know this per-section, and triples seem to be the only thing easily available (and there, windows-coff implies thumb).


https://reviews.llvm.org/D22776