1. Aug 30, 2020
  2. Aug 29, 2020
    • Roman Lebedev's avatar
      [InstCombine] Take 3: Perform trivial PHI CSE · bf21ce7b
      Roman Lebedev authored
      The original take 1 was 6102310d,
      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 ed90f15e
      that reverted it:
      > It appears to cause compilation non-determinism and caused stage3 mismatches.
      
      Then there was take 2 3e69871a,
      which was InstCombine-specific, but it again showed stage2-stage3 differences,
      and reverted in bdaa3f86.
      This is quite alarming.
      
      Here, let's try to change how we find existing PHI candidate:
      due to the worklist order, and the way PHI nodes are inserted
      (it may be inserted as the first one, or maybe not), let's look at *all*
      PHI nodes in the block.
      
      Effects on vanilla llvm test-suite + RawSpeed:
      ```
      | statistic name                                     | baseline  | proposed  |      Δ |        % |    \|%\| |
      |----------------------------------------------------|-----------|-----------|-------:|---------:|---------:|
      | asm-printer.EmittedInsts                           | 7942329   | 7942457   |    128 |    0.00% |    0.00% |
      | assembler.ObjectBytes                              | 254295632 | 254312480 |  16848 |    0.01% |    0.01% |
      | correlated-value-propagation.NumPhis               | 18412     | 18347     |    -65 |   -0.35% |    0.35% |
      | early-cse.NumCSE                                   | 2183283   | 2183267   |    -16 |    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   | 3644419   |   4108 |    0.11% |    0.11% |
      | instcombine.NumDeadInst                            | 1778204   | 1783205   |   5001 |    0.28% |    0.28% |
      | instcombine.NumPHICSEs                             | 0         | 22490     |  22490 |    0.00% |    0.00% |
      | instcombine.NumWorklistIterations                  | 2023272   | 2024400   |   1128 |    0.06% |    0.06% |
      | instcount.NumCallInst                              | 1758395   | 1758802   |    407 |    0.02% |    0.02% |
      | instcount.NumInvokeInst                            | 59478     | 59502     |     24 |    0.04% |    0.04% |
      | instcount.NumPHIInst                               | 330557    | 330545    |    -12 |    0.00% |    0.00% |
      | instcount.TotalBlocks                              | 1077138   | 1077220   |     82 |    0.01% |    0.01% |
      | instcount.TotalFuncs                               | 101442    | 101441    |     -1 |    0.00% |    0.00% |
      | instcount.TotalInsts                               | 8831946   | 8832606   |    660 |    0.01% |    0.01% |
      | simplifycfg.NumHoistCommonCode                     | 24186     | 24187     |      1 |    0.00% |    0.00% |
      | simplifycfg.NumInvokes                             | 4300      | 4410      |    110 |    2.56% |    2.56% |
      | simplifycfg.NumSimpl                               | 1019813   | 999767    | -20046 |   -1.97% |    1.97% |
      ```
      So it fires 22490 times, which is less than ~24k the take 1 did,
      but more than what take 2 did (22228 times)
      .
      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.
      bf21ce7b
    • sstefan1's avatar
    • Roman Lebedev's avatar
      Revert "[InstCombine] Take 2: Perform trivial PHI CSE" · bdaa3f86
      Roman Lebedev authored
      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 3e69871a.
      bdaa3f86
    • Nikita Popov's avatar
      [InstCombine] Return replaceInstUsesWith() result (NFC) · 6093b14c
      Nikita Popov authored
      Follow the usual usage pattern for this function and return the
      result.
      6093b14c
    • Martin Storsjö's avatar
      [AArch64] Generate and parse SEH assembly directives · 5b86d130
      Martin Storsjö authored
      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
      5b86d130
    • Martin Storsjö's avatar
      [MC] [Win64EH] Fill in FuncletOrFuncEnd if missing · 20f7773b
      Martin Storsjö authored
      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
      20f7773b
    • Roman Lebedev's avatar
      [InstCombine] foldAggregateConstructionIntoAggregateReuse(): use... · 71ac9105
      Roman Lebedev authored
      [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.
      71ac9105
    • Roman Lebedev's avatar
      [InstCombine] canonicalizeICmpPredicate(): use InstCombiner::replaceInstUsesWith() instead of RAUW · e65f2131
      Roman Lebedev authored
      We really shouldn't use RAUW in InstCombine
      because we should consistently update Worklist to avoid extra iterations.
      e65f2131