In D72449#1812818, @arichardson wrote:

I'm fine with this workaround although I'm very surprised that the test is not working. Especially since deterministic-archive.test and replace-update.test also set TZ to get reproducible output

Do you have access to any of the failing bots? If so what happens there if you change the TZ variable?
I get the following on all systems I have access to when I run something like date && date -u && env TZ=CET date && env TZ=CET date -u && env TZ=BST date && env TZ=BST date -u I get the following output:

Thu  9 Jan 2020 20:18:10 GMT
Thu  9 Jan 2020 20:18:10 UTC
Thu  9 Jan 2020 21:18:10 CET
Thu  9 Jan 2020 20:18:10 UTC
Thu  9 Jan 2020 20:18:10 UTC
Thu  9 Jan 2020 20:18:10 UTC

In D72449#1812130, @arichardson wrote:

I was careful in the test to use TZ=UTC for all commands printing dates and it works fine for me in UTC+1 and in the UK which is currently UTC. Does the bot in question ignore the TZ variable?

Updated patch based on previous comments.

I am investigating a recurring regression on arm-linux bots and bisecting the commits on the build [1] that introduced the regression it points to this one. By reverting I can build the stage1 (I am testing now the stage2). I seems to only happen on arm-linux bots, I didn't see it on aarch64-linux ones.

I am investigating a recurring regression on aarch64-linux bots and bisecting the commits on the build [1] that introduced the regression it points to this one. I don't understand exactly what is triggering the issue, but it only happen on the stage2 build with stage1 clang built with different compilers (gcc 5.4, gcc-7.4.0, and gcc-8.3.0). The issue is on clang itself while trying to execute the Analysis/uninit-sometimes.cpp test:

I checked it with a gdb built against python 2.7.2 and python 3.7.4. I still seeing some issue on the arm bot, but I think it is unrelated to the python3 support now.

Ping.

In D67733#1674557, @saugustine wrote:

Thanks so much for fixing this. Really appreciate it.

Ping.

Is this patch ok with windows support now?

Updated patch based on previous comments. Add a pattern to handle i32 return and a testcase for Windows.

Ping now that both D62017 and D62019 have been approved.

Updated patch based on D62026.

Updated patch based on D62026.

In D62017#1505300, @craig.topper wrote:

Can we do this with 2 intrinsics with overloaded result types as I've done for lround/llround in D62026?

LGTM, thanks for the follow-up patch.

So should I handle f16 in a different patch or should I also adapt it on this one?

Ping again now that D61390 has been approved.

Ping.

Updated patch based on the previous comment. The main change is the IBM long double tests which also required both SoftenFloatOp_LROUND and SoftenFloatOp_LLROUND implementation to handle the type correctly.

In D61390#1495541, @craig.topper wrote:

Should we have a PowerPC test for ppcf128?

Updated patch based on previous review.

Updated patch based on previous comments. The changes are:

In D61390#1492891, @craig.topper wrote:

Can you test lround.i32 with -mtriple=i686-unknown with and without -mattr=sse2.

Use utils/update_llc_test_checks.py to generate the checks for at least X86. Try it on the other targets too if it works for them.

Updated patch based on the previous comment. The main change are:

Updated patch based on D61390 changes. The main change is it allows
index both lround/llround by returned type.

Updated patch based on the previous comment. The main change adapt the testcases for D61390
update and handle lroundl/llroundl correctly.

In fact, it turned out that indexing the new lround/llround based on
input argument didn't really make it correctly handled as an expanded
operation. I will dig into exactly why backend is not selecting correctly
based on input argument, so I change it back to previous indexing by
return type.

Updated patch based on previous comments. The changes from the previous version are:

Updated patch based on previous comments.

Ping, only this refactor is missing review for my aarch64 fp materialization optimization.

Ping.

Ping.

Fxied CamelCase (sorry missing the comment).

I added a testcase based on test/CodeGen/ARM/immcost.ll.

Updated patch based on previous comments.

Updated patch based on previous comments. It depends on https://reviews.llvm.org/D58915. I am trying to come up with a testcase to actually test it. The the main difference for the cost analysis is just for some immediate that contains some 16-bit zero or one chunk (which will have the size adjusted from 3 to 2). Since they are not TCC_Free, TCC_Basic, or TCC_Expensive the change does not actually interfere in the further cost analysis.

Updated patch based on previous comments. It depends on https://reviews.llvm.org/D58915 and https://reviews.llvm.org/D58690

Updated patch based on previous comments.

In D58460#1411040, @efriedma wrote:

Not sure I like the duplicated logic in getExpandImmCost; it doesn't have good test coverage, and it could fall out of sync in the future. That said, I've been considering refactoring the code in expandMOVImm anyway, to split the actual instruction emission away from the logic that figures out the appropriate sequence. Basically, the idea would be that instead of returning a number from getExpandImmCost, you return an abstraction of the instruction sequence: an array that contains, for each instruction, the appropriate opcode and immediate. isFPImmLegal just uses the number of elements in the array, while expandMOVImm actually emits instructions based on the array. I think this would shrink the code overall because the logic for building instructions is currently duplicated multiple times. (I was considering it more in the context of adding more possible sequences, but it works here as well.)

New revision based on previous comments. I refactored the logic used on isFPImmLegal to evaluate whether to materialize the FP constant or not by adding a new function on common aarch64 code, AArch64_AM::getExpandImmCost. To avoid code replication I refactored the code by moving some definitions from AArch64ExpandPseudoInsts.cpp.

Updated patch based on previous comment:

Ping.

Patch updated based on previous comments. The isAnyMOVWMovAlias
can catch more cases where we can materialize the floating-point constant
than isLogicalImmediate (128 .0 for instance). The positive zero still need to
be handled as specific case because the resulting fmov will use the zero
register instead of an immediate. The isAnyMOVWMovAlias path is not use
for fp16 (I will need to check further if is safe for all cases).

Updated patch from previous comments. I added an extra check using AArch64_AM::isLogicalImmediate for constants that can be materialized as the immediate operand of a logical instruction. We still need to handle the cases for some 8-bit immediates on fmov using getFPXXImm and positive 0.0.

In D57044#1367943, @efriedma wrote:

Is there some reason we're checking for specific floating-point constants here, as opposed to just calling AArch64_AM::isLogicalImmediate or AArch64_AM::isAnyMOVWMovAlias?

Updated patch with additional tests. I added tests for __builtin_isinf expanded builtin for float, double, and long double (the latter still requires loading the constant).