SuccessChanges

Summary

  1. [NFC][STLExtras] Add make_first_range(), similar to existing make_second_range() (details)
  2. [NFC][InstructionSimplify] Add a warning about not simplifying to not def-reachable (details)
  3. [Attributes] Merge calls to getFnAttribute/hasFnAttribute using Attribute::isValid. NFC (details)
  4. [Target][AArch64] Allow for char as int8_t in AArch64AsmParser.cpp (details)
  5. [InstCombine] Fix typo in comment (NFC) (details)
  6. [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE (details)
  7. [NFC][InstCombine] Add tests for PHI CSE (details)
  8. [NFC][InstSimplify] Add a note to PHI CSE tests that they are all negative tests (details)
  9. [InstCombine] Take 2: Perform trivial PHI CSE (details)
  10. [InstCombine] visitPHINode(): use InstCombiner::replaceInstUsesWith() instead of RAUW (details)
  11. [NFC][InstCombine] select.ll: remove outdated TODO comment (details)
  12. [NFC][InstCombine] Add STATISTIC() for how many iterations we did (details)
  13. [NFC] Instruction::isIdenticalToWhenDefined(): s/nessesairly/necessarily/ (details)
  14. [NFC][InstCombine] Fix some comments: the code already uses IC::replaceInstUsesWith() (details)
  15. [InstCombine] canonicalizeICmpPredicate(): use InstCombiner::replaceInstUsesWith() instead of RAUW (details)
  16. [InstCombine] foldAggregateConstructionIntoAggregateReuse(): use InstCombiner::replaceInstUsesWith() instead of RAUW (details)
  17. [MC] [Win64EH] Fill in FuncletOrFuncEnd if missing (details)
  18. [AArch64] Generate and parse SEH assembly directives (details)
  19. [InstCombine] Return replaceInstUsesWith() result (NFC) (details)
  20. Revert "[InstCombine] Take 2: Perform trivial PHI CSE" (details)
  21. [UpdateTestChecks] Don't skip attributes when comparing functions (details)
Commit 08669fbb439d139e2a66d2ae120a4b5be4fb4f28 by lebedev.ri
[NFC][STLExtras] Add make_first_range(), similar to existing make_second_range()

Having just one of the two seens weird.
I wanted to use it a few times, but it wasn't there.
The file was modifiedllvm/include/llvm/ADT/STLExtras.h
Commit c1b3e32118adff13bf846e0aa8b0b3b4ec04a120 by lebedev.ri
[NFC][InstructionSimplify] Add a warning about not simplifying to not def-reachable

See
https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200824/824235.html
and
https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200824/824967.html

InstSimply is not allowed to perform simplifications to instructions
that are not def-reachable from the original instruction.
The file was modifiedllvm/lib/Analysis/InstructionSimplify.cpp
The file was modifiedllvm/include/llvm/Analysis/InstructionSimplify.h
Commit 6dcd9f517e96a0ed418539b55decd10aae014752 by craig.topper
[Attributes] Merge calls to getFnAttribute/hasFnAttribute using Attribute::isValid. NFC

Rather than calling hasFnAttribute and then calling getFnAttribute
if the attribute exists, its better to just call getFnAttribute and
then check if we got a valid attribute back.
The file was modifiedllvm/lib/IR/Attributes.cpp
Commit 672d7836bbf08be0f1d4421bae23d8d0689c739c by ro
[Target][AArch64] Allow for char as int8_t in AArch64AsmParser.cpp

A couple of AArch64 tests were failing on Solaris, both sparc and x86:

  LLVM :: MC/AArch64/SVE/add-diagnostics.s
  LLVM :: MC/AArch64/SVE/cpy-diagnostics.s
  LLVM :: MC/AArch64/SVE/cpy.s
  LLVM :: MC/AArch64/SVE/dup-diagnostics.s
  LLVM :: MC/AArch64/SVE/dup.s
  LLVM :: MC/AArch64/SVE/mov-diagnostics.s
  LLVM :: MC/AArch64/SVE/mov.s
  LLVM :: MC/AArch64/SVE/sqadd-diagnostics.s
  LLVM :: MC/AArch64/SVE/sqsub-diagnostics.s
  LLVM :: MC/AArch64/SVE/sub-diagnostics.s
  LLVM :: MC/AArch64/SVE/subr-diagnostics.s
  LLVM :: MC/AArch64/SVE/uqadd-diagnostics.s
  LLVM :: MC/AArch64/SVE/uqsub-diagnostics.s

For example, reduced from `MC/AArch64/SVE/add-diagnostics.s`:

  add     z0.b, z0.b, #0, lsl #8

missed the expected diagnostics

  $ ./bin/llvm-mc -triple=aarch64 -show-encoding -mattr=+sve add.s
  add.s:1:21: error: immediate must be an integer in range [0, 255] with a shift amount of 0
  add     z0.b, z0.b, #0, lsl #8
                      ^

The message is `Match_InvalidSVEAddSubImm8`, emitted in the generated
`lib/Target/AArch64/AArch64GenAsmMatcher.inc` for `MCK_SVEAddSubImm8`.
When comparing the call to `::AArch64Operand::isSVEAddSubImm<char>` on both
Linux/x86_64 and Solaris, I find

  875     bool IsByte = std::is_same<int8_t, std::make_signed_t<T>>::value;

is `false` on Solaris, unlike Linux.

The problem boils down to the fact that `int8_t` is plain `char` on
Solaris: both the sparc and i386 psABIs have `char` as signed.  However,
with

  9887     DiagnosticPredicate DP(Operand.isSVEAddSubImm<int8_t>());

in `lib/Target/AArch64/AArch64GenAsmMatcher.inc`, `std::make_signed_t<int8_t>`
above yieds `signed char`, so `std::is_same<int8_t, signed char>` is `false`.

This can easily be fixed by also allowing for `int8_t` here and in a few
similar places.

Tested on `amd64-pc-solaris2.11`, `sparcv9-sun-solaris2.11`, and
`x86_64-pc-linux-gnu`.

Differential Revision: https://reviews.llvm.org/D85225
The file was modifiedllvm/lib/Target/AArch64/MCTargetDesc/AArch64AddressingModes.h
The file was modifiedllvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
Commit 57a26bb7b43556f255be1bf005e7acd8e2f6eae2 by nikita.ppv
[InstCombine] Fix typo in comment (NFC)

As pointed out in post-commit review of D63060.
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
Commit fc2dac4116df62c25396b15ec9730f67a0b1aac7 by dimitry
[PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

As a prerequisite to doing experimental buids of pieces of FreeBSD PowerPC64 as little-endian, allow actually targeting it.

This is needed so basic platform definitions are pulled in. Without it, the compiler will only run freestanding.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D73425
The file was modifiedclang/test/Preprocessor/init-ppc64.c
The file was modifiedclang/test/Driver/freebsd.c
The file was modifiedclang/test/CodeGen/target-data.c
The file was modifiedclang/lib/Basic/Targets.cpp
The file was modifiedclang/test/Driver/ppc-abi.c
Commit 3ba83f2d29e5103b864a5a2f957bd10be73d9663 by lebedev.ri
[NFC][InstCombine] Add tests for PHI CSE

As discussed in https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200824/824235.html
even though it seems worthwhile doing so in InstSimplify,
we really can't do that there, because the other PHI wouldn't be
def-reachable from the original PHI.

So ignoring whether or not EarlyCSE should also know to do this,
InstCombine is the place.
The file was addedllvm/test/Transforms/InstCombine/phi-cse.ll
Commit aea042e978bf7b7af95336916750b4f737044291 by lebedev.ri
[NFC][InstSimplify] Add a note to PHI CSE tests that they are all negative tests

As discussed in https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20200824/824235.html
even though it seems worthwhile doing so in InstSimplify,
we really can't do that there, because the other PHI wouldn't be
def-reachable from the original PHI.
The file was modifiedllvm/test/Transforms/InstSimplify/phi-cse.ll
Commit 3e69871ab5a66fb55913a2a2f5e7f5b42899a4c9 by lebedev.ri
[InstCombine] Take 2: Perform trivial PHI CSE

The original take was 6102310d814ad73eab60a88b21dd70874f7a056f,
which taught InstSimplify to do that, which seemed better at time,
since we got EarlyCSE support for free.

However, it was proven that we can not do that there,
the simplified-to PHI would not be reachable from the original PHI,
and that is not something InstSimplify is allowed to do,
as noted in the commit ed90f15efb40d26b5d3ead3bb8e9e284218e0186
that reverted it :
> It appears to cause compilation non-determinism and caused stage3 mismatches.

However InstCombine already does many different optimizations,
so it should be a safe place to do it here.

Note that we still can't just compare incoming values ranges,
because there is no guarantee that these PHI's we'd simplify to
were already re-visited and sorted.
However coming up with a test is problematic.

Effects on vanilla llvm test-suite + RawSpeed:
```
| statistic name                                     | baseline  | proposed  |      Δ |        % |      |%| |
|----------------------------------------------------|-----------|-----------|-------:|---------:|---------:|
| instcombine.NumPHICSEs                             | 0         | 22228     |  22228 |    0.00% |    0.00% |
| asm-printer.EmittedInsts                           | 7942329   | 7942456   |    127 |    0.00% |    0.00% |
| assembler.ObjectBytes                              | 254295632 | 254313792 |  18160 |    0.01% |    0.01% |
| early-cse.NumCSE                                   | 2183283   | 2183272   |    -11 |    0.00% |    0.00% |
| early-cse.NumSimplify                              | 550105    | 541842    |  -8263 |   -1.50% |    1.50% |
| instcombine.NumAggregateReconstructionsSimplified  | 73        | 4506      |   4433 | 6072.60% | 6072.60% |
| instcombine.NumCombined                            | 3640311   | 3666911   |  26600 |    0.73% |    0.73% |
| instcombine.NumDeadInst                            | 1778204   | 1783318   |   5114 |    0.29% |    0.29% |
| instcount.NumCallInst                              | 1758395   | 1758804   |    409 |    0.02% |    0.02% |
| instcount.NumInvokeInst                            | 59478     | 59502     |     24 |    0.04% |    0.04% |
| instcount.NumPHIInst                               | 330557    | 330549    |     -8 |    0.00% |    0.00% |
| instcount.TotalBlocks                              | 1077138   | 1077221   |     83 |    0.01% |    0.01% |
| instcount.TotalFuncs                               | 101442    | 101441    |     -1 |    0.00% |    0.00% |
| instcount.TotalInsts                               | 8831946   | 8832611   |    665 |    0.01% |    0.01% |
| simplifycfg.NumInvokes                             | 4300      | 4410      |    110 |    2.56% |    2.56% |
| simplifycfg.NumSimpl                               | 1019813   | 999740    | -20073 |   -1.97% |    1.97% |
```
So it fires ~22k times, which is less than ~24k the take 1 did.
It allows foldAggregateConstructionIntoAggregateReuse() to actually work
after PHI-of-extractvalue folds did their thing. Previously SimplifyCFG
would have done this PHI CSE, of all places. Additionally, allows some
more `invoke`->`call` folds to happen (+110, +2.56%).

All in all, expectedly, this catches less things overall,
but all the motivational cases are still caught, so all good.
The file was modifiedllvm/test/Transforms/InstCombine/merging-multiple-stores-into-successor.ll
The file was modifiedllvm/test/Transforms/InstCombine/phi-cse.ll
The file was modifiedllvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
The file was modifiedllvm/test/Transforms/LoopVectorize/reduction.ll
The file was modifiedllvm/lib/IR/Instruction.cpp
The file was modifiedllvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
The file was modifiedllvm/test/Transforms/InstCombine/select.ll
Commit 4f4eecf0ec56a15b3dc377cac3799463cbf77a1f by lebedev.ri
[InstCombine] visitPHINode(): use InstCombiner::replaceInstUsesWith() instead of RAUW

As noted in post-commit review, we really shouldn't use RAUW in InstCombine
because we should consistently update Worklist to avoid extra iterations.
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
Commit 9c1ca8a3f7c5bda04405da761c7a59b1d5b1a129 by lebedev.ri
[NFC][InstCombine] select.ll: remove outdated TODO comment

Fixed by 3e69871ab5a66fb55913a2a2f5e7f5b42899a4c9
The file was modifiedllvm/test/Transforms/InstCombine/select.ll
Commit 49d223274f69bd67fe6e24535e3a2f68182f011b by lebedev.ri
[NFC][InstCombine] Add STATISTIC() for how many iterations we did

As we've established, if it takes more than two iterations
(one to perform folding and one to ensure that no folding opportunities
remain) per function, then there are worklist management issues.
So it may be interesting to keep track of it.
The file was modifiedllvm/lib/Transforms/InstCombine/InstructionCombining.cpp
Commit 65b3854e106e30b88f6b10e62a3448ac953b797a by lebedev.ri
[NFC] Instruction::isIdenticalToWhenDefined(): s/nessesairly/necessarily/
The file was modifiedllvm/lib/IR/Instruction.cpp
Commit bd12113f575be38b6b3d65fa599ac6bb84918689 by lebedev.ri
[NFC][InstCombine] Fix some comments: the code already uses IC::replaceInstUsesWith()
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
Commit e65f2131782bc39ec66fc9c74991ff6c2ff1d93b by lebedev.ri
[InstCombine] canonicalizeICmpPredicate(): use InstCombiner::replaceInstUsesWith() instead of RAUW

We really shouldn't use RAUW in InstCombine
because we should consistently update Worklist to avoid extra iterations.
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombineInternal.h
Commit 71ac9105cd392a257d15515579c70f06c970c5cb by lebedev.ri
[InstCombine] foldAggregateConstructionIntoAggregateReuse(): use InstCombiner::replaceInstUsesWith() instead of RAUW

We really shouldn't use RAUW in InstCombine
because we should consistently update Worklist to avoid extra iterations.
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
Commit 20f7773bb4bb458c3f3b4e46d1601465e6479746 by martin
[MC] [Win64EH] Fill in FuncletOrFuncEnd if missing

This can happen e.g. for code that declare .seh_proc/.seh_endproc
in assembly, or for code that use .seh_handlerdata (which triggers
the unwind info to be emitted before the end of the function).

The TextSection field must be made non-const to be able to use it
with Streamer.SwitchSection().

Differential Revision: https://reviews.llvm.org/D86528
The file was modifiedllvm/include/llvm/MC/MCWinEH.h
The file was modifiedllvm/lib/MC/MCWin64EH.cpp
The file was modifiedllvm/test/MC/AArch64/seh.s
The file was modifiedllvm/lib/MC/MCStreamer.cpp
Commit 5b86d130e2baed7221b09087c506f5974fe65f22 by martin
[AArch64] Generate and parse SEH assembly directives

This ensures that you get the same output regardless if generating
code directly to an object file or if generating assembly and
assembling that.

Add implementations of the EmitARM64WinCFI*() methods in
AArch64TargetAsmStreamer, and fill in one blank in MCAsmStreamer.

Add corresponding directive handlers in AArch64AsmParser and
COFFAsmParser.

Some SEH directive names have been picked to match the prior art
for SEH assembly directives for x86_64, e.g. the spelling of
".seh_startepilogue" matching the preexisting ".seh_endprologue".

For the directives for saving registers, the exact spelling
from the arm64 documentation is picked, e.g. ".seh_save_reg" (to follow
that naming for all the other ones, e.g. ".seh_save_fregp_x"), while
the corresponding one for x86_64 is plain ".seh_savereg" without the
second underscore.

Directives in the epilogues have the same names as in prologues,
e.g. .seh_savereg, even though the registers are restored, not
saved, at that point.

Differential Revision: https://reviews.llvm.org/D86529
The file was modifiedllvm/test/CodeGen/AArch64/lround-conv-win.ll
The file was modifiedllvm/test/CodeGen/AArch64/wineh1.mir
The file was modifiedllvm/lib/Target/AArch64/MCTargetDesc/AArch64ELFStreamer.cpp
The file was modifiedllvm/test/CodeGen/AArch64/win_cst_pool.ll
The file was modifiedllvm/test/CodeGen/AArch64/lrint-conv-win.ll
The file was modifiedllvm/lib/MC/MCAsmStreamer.cpp
The file was modifiedllvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
The file was modifiedllvm/test/CodeGen/AArch64/lround-conv-fp16-win.ll
The file was modifiedllvm/test/CodeGen/AArch64/wineh-try-catch-nobase.ll
The file was modifiedllvm/test/CodeGen/AArch64/windows-extern-weak.ll
The file was modifiedllvm/test/CodeGen/AArch64/wineh-try-catch-vla.ll
The file was modifiedllvm/test/CodeGen/AArch64/wineh-try-catch-realign.ll
The file was modifiedllvm/test/CodeGen/AArch64/lrint-conv-fp16-win.ll
The file was modifiedllvm/lib/MC/MCParser/COFFAsmParser.cpp
The file was modifiedllvm/test/CodeGen/AArch64/powi-windows.ll
The file was modifiedllvm/test/CodeGen/AArch64/arm64-windows-calls.ll
The file was modifiedllvm/test/CodeGen/AArch64/seh_funclet_x1.ll
The file was modifiedllvm/test/CodeGen/AArch64/win64-jumptable.ll
The file was modifiedllvm/test/CodeGen/AArch64/wineh-try-catch-cbz.ll
The file was modifiedllvm/test/MC/AArch64/seh.s
Commit 6093b14c2c1fc785bfb7040ecebfe4127e1ab9ab by nikita.ppv
[InstCombine] Return replaceInstUsesWith() result (NFC)

Follow the usual usage pattern for this function and return the
result.
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
Commit bdaa3f86a040b138c58de41d73d35b76fdec1380 by lebedev.ri
Revert "[InstCombine] Take 2: Perform trivial PHI CSE"

While the original variant with doing this in InstSimplify (rightfully)
caused questions and ultimately was detected to be a culprit
of stage2-stage3 mismatch, it was expected that
InstCombine-based implementation would be fine.

But apparently it's not, as
http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/24095/steps/compare-compilers/logs/stdio
suggests.

Which suggests that somewhere in InstCombine there is a loop
over nondeterministically sorted container, which causes
different worklist ordering.

This reverts commit 3e69871ab5a66fb55913a2a2f5e7f5b42899a4c9.
The file was modifiedllvm/test/Transforms/InstCombine/merging-multiple-stores-into-successor.ll
The file was modifiedllvm/test/Transforms/InstCombine/phi-cse.ll
The file was modifiedllvm/test/Transforms/InstCombine/phi-equal-incoming-pointers.ll
The file was modifiedllvm/test/Transforms/InstCombine/select.ll
The file was modifiedllvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
The file was modifiedllvm/test/Transforms/LoopVectorize/reduction.ll
The file was modifiedllvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
Commit b5dd2cd27f6206a027a1c5b572922a9d4f9891d9 by sstipanovic
[UpdateTestChecks] Don't skip attributes when comparing functions
The file was modifiedllvm/utils/UpdateTestChecks/common.py