[PATCH] D108514: [TargetMachine] Move COFF special case for ExternalSymbolSDNode from shouldAssumeDSOLocal to X86Subtarget

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 23 03:09:38 PDT 2021


mstorsjo added a comment.

> Intended to be NFC. ARM/AArch64 don't appear to need adjustment.

Hmm, it seems like the COFF specific code that this refers to has gone through a couple refactorings since I touched it, so I don't remember exactly which things it refers to and when that check was needed. My main concern is if the conclusion that the ARM/AArch64 codepaths don't need adjustment comes from some supposedly COFF-generic test only is executed for x86.

It's also plausible that it just isn't needed there, because those corresponding arm/aarch64 subtarget functions for classifying references are indeed a little bit different than the x86 one.

In practice, I did include this patch in my nightly test run (running things for all 4 architectures, which should try out many of the mingw specific edge cases) and didn't run into any issues with it - so I guess it should be practically fine...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108514/new/

https://reviews.llvm.org/D108514



More information about the llvm-commits mailing list