[PATCH] D93625: [NFC] [TEST] Fix the threads.ll for Windows

Nigel Perks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 9 02:58:32 PST 2021


nigelp-xmos added a comment.

I think this patch can be cancelled, because the issue is already fixed in the main branch. But please say if I have missed something.

Test threads.ll started failing on Windows in commit https://github.com/llvm/llvm-project/commit/961f31d8ad14c66829991522d73e14b5a96ff6d4 . That commit changed function TargetMachine::shouldAssumeDSOLocal(). Previously, for both Windows and non-Windows, in f_tle in this test, in the DAG combine phase, for global tle:

TargetMachine::shouldAssumeDSOLocal returned true to:
TargetLowering::isOffsetFoldingLegal which returned true to:
SelectionDAG::isConstantIntBuildVectorOrConstantInt which returned global address node to:
DAGCombiner::visitADDLike which put the operands in a certain order.

With the new code, for non-Windows, in f_tle, shouldAssumeDSOLocal returns false, and the operands are not reordered in that way. But for Windows, the function still returns true before the new code is reached, because of special case:

if (TT.isOSBinFormatCOFF() || TT.isOSWindows()) return true;

So the new code produces different output for Windows and non-Windows.

Since we do not need to target Windows on XCore, we do not need to check for specific Windows results, and can use -mtriple to set OS unknown. So I think the existing solution in branch main already resolves the issue of the difference on Windows which this patch is addressing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93625



More information about the llvm-commits mailing list