@epastor This is causing build bots failures, please can you take a look? http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/3121

This breaks tests on non-Win, see e.g. http://45.33.8.238/linux/7846/step_7.txt http://45.33.8.238/mac/6120/step_7.txt

I agree that adjusting BBDuplicateThreshold for code size is better choice than disabling the whole pass.

Thanks, you both are indeed right that it can reduce code size... I've now tried reducing BBDuplicateThreshold and it looks like halving this to 3 is the best option (for aarch64 at least). There's a couple of outliers, but most changes are -0.1%:
test-suite...otout/Shootout-nestedloop.test 484 468 -3.3%
test-suite...+/Shootout-C++-nestedloop.test 532 516 -3.0%
test-suite...Olden/perimeter/perimeter.test 2108 2148 1.9%
test-suite...urce/Applications/aha/aha.test 2964 2932 -1.1%
test-suite...ce/Benchmarks/PAQ8p/paq8p.test 53748 53964 0.4%
test-suite...ch/g721/g721encode/encode.test 5468 5452 -0.3%
test-suite...marks/Ptrdist/yacr2/yacr2.test 15204 15236 0.2%
test-suite...ications/JM/ldecod/ldecod.test 161116 160788 -0.2%
test-suite...marks/7zip/7zip-benchmark.test 469784 470728 0.2%
test-suite...chmarks/MallocBench/gs/gs.test 104228 104428 0.2%
test-suite...oxyApps-C/miniAMR/miniAMR.test 34260 34196 -0.2%
test-suite...ications/JM/lencod/lencod.test 380324 380996 0.2%
test-suite...plications/d/make_dparser.test 62816 62912 0.2%
test-suite...s-C/Pathfinder/PathFinder.test 15996 15972 -0.2%
test-suite...ks/Prolangs-C++/city/city.test 5428 5436 0.1%
test-suite.../Benchmarks/Ptrdist/bc/bc.test 27300 27260 -0.1%
test-suite...pplications/oggenc/oggenc.test 105776 105632 -0.1%
test-suite...-typeset/consumer-typeset.test 327944 327520 -0.1%
test-suite...lications/sqlite3/sqlite3.test 244704 244400 -0.1%
test-suite...tions/lambda-0.1.3/lambda.test 20092 20068 -0.1%
test-suite.../Prolangs-C/bison/mybison.test 35828 35788 -0.1%
test-suite...peg2/mpeg2dec/mpeg2decode.test 29916 29884 -0.1%
test-suite.../Applications/SPASS/SPASS.test 287832 288136 0.1%
test-suite...arching-flt/Searching-flt.test 8292 8284 -0.1%
test-suite...arching-dbl/Searching-dbl.test 8500 8492 -0.1%
test-suite...C/Packing-flt/Packing-flt.test 8548 8540 -0.1%
test-suite...ences-flt/Recurrences-flt.test 8644 8636 -0.1%
test-suite...t/StatementReordering-flt.test 8660 8652 -0.1%
test-suite...encode/alacconvert-encode.test 26044 26020 -0.1%
test-suite...decode/alacconvert-decode.test 26044 26020 -0.1%
test-suite...C/Packing-dbl/Packing-dbl.test 8756 8748 -0.1%
test-suite...lications/SIBsim4/SIBsim4.test 26476 26500 0.1%
test-suite...ences-dbl/Recurrences-dbl.test 8852 8844 -0.1%
test-suite...l/StatementReordering-dbl.test 8868 8860 -0.1%
test-suite...ing-flt/LoopRerolling-flt.test 9116 9108 -0.1%
test-suite...ing-flt/Equivalencing-flt.test 9268 9260 -0.1%
test-suite...ing-flt/NodeSplitting-flt.test 9276 9268 -0.1%
test-suite...ing-dbl/LoopRerolling-dbl.test 9340 9332 -0.1%
test-suite...mbolics-flt/Symbolics-flt.test 9364 9356 -0.1%
test-suite...ing-dbl/Equivalencing-dbl.test 9420 9412 -0.1%
test-suite...ing-dbl/NodeSplitting-dbl.test 9476 9468 -0.1%
test-suite...mbolics-dbl/Symbolics-dbl.test 9588 9580 -0.1%
test-suite...lt/IndirectAddressing-flt.test 9708 9700 -0.1%
test-suite...bl/IndirectAddressing-dbl.test 9924 9916 -0.1%
test-suite...lt/CrossingThresholds-flt.test 9940 9932 -0.1%
test-suite...cations/hexxagon/hexxagon.test 10124 10132 0.1%
test-suite...bl/CrossingThresholds-dbl.test 10148 10140 -0.1%
test-suite...flt/InductionVariable-flt.test 10204 10196 -0.1%
test-suite...flt/LoopRestructuring-flt.test 10436 10428 -0.1%
test-suite...ow-flt/GlobalDataFlow-flt.test 10452 10444 -0.1%
test-suite...dbl/InductionVariable-dbl.test 10468 10460 -0.1%
test-suite...oops-flt/ControlLoops-flt.test 10588 10580 -0.1%
test-suite...dbl/LoopRestructuring-dbl.test 10644 10636 -0.1%
test-suite...ow-dbl/GlobalDataFlow-dbl.test 10660 10652 -0.1%
test-suite...oops-dbl/ControlLoops-dbl.test 10780 10772 -0.1%
test-suite...pansion-flt/Expansion-flt.test 10996 10988 -0.1%
test-suite...-flt/LinearDependence-flt.test 11148 11140 -0.1%
test-suite...pansion-dbl/Expansion-dbl.test 11196 11188 -0.1%
test-suite...ctions-flt/Reductions-flt.test 11316 11308 -0.1%
test-suite...-dbl/LinearDependence-dbl.test 11364 11356 -0.1%
test-suite...ctions-dbl/Reductions-dbl.test 11524 11516 -0.1%
test-suite...sc-C++/stepanov_container.test 13132 13140 0.1%
test-suite...lFlow-flt/ControlFlow-flt.test 13764 13756 -0.1%
test-suite...lFlow-dbl/ControlFlow-dbl.test 13972 13964 -0.1%
Geomean difference -0.0%

default-results    threshold-3        diff

count 310.000000 310.000000 310.000000
mean 26375.587097 26377.445161 -0.000273
std 65478.305060 65501.068235 0.002870
min 460.000000 460.000000 -0.033058
25% 1286.000000 1286.000000 0.000000
50% 3128.000000 3128.000000 0.000000
75% 11262.000000 11260.000000 0.000000
max 469784.000000 470728.000000 0.018975

Rebase. Regenerate with a hacked script to show the real stack offsets for reviewing. I'll regenerate before commiting so the next person who runs the script won't get a surprise.

yeah, here one example

inttoptr and ptrtoint should be treated as opaque ones. If needed, they should be handled with target-specific. This pass is definitely not the place to handle them.

In D72463#1826821, @MaskRay wrote:

In D72463#1826499, @annita.zhang wrote:

@MaskRay Did you merge it to LLVM 10 branch?

It is included in the release branch.

git branch origin/release/10.x --contains 5ca24d09aefaedf8e4148c7fce4b4ab0c4ecc72a # suceeded

@tstellar Thanks for your comments. The MachineInstr SSA based optimization is really a ugly hack. Currently verifier is not able to verify the program correctly, i.e., rejects the program for certain patterns related subregister uses. The commit message provides more information. @ast suggested to look at whether we can improve kernel verifier and I will take a look there. If kernel can be enhanced, I will drop the second MachineInstr SSA pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

I may not quite understand what the verifier is looking for here, but rather than trying to undo the optimization in 2 different places, would it be possible to implement this by having the InstructionSelector emit some kind of Pseudo Instruction for inputs to the comparison? For example, what would normally be selected to:

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Removing space

Thanks River for the review. Addressed your comments.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Testing the change by adapting existing test

In D72961#1827862, @efriedma wrote:

My initial reaction was that the PHI was invalid, but according to @qcolombet it's ok to have different register banks on the operands

Okay... but the operand register class can't actually be *anything*, can it? We have restrictions on COPY instructions.

Those restrictions are target specific though aren’t they? I think it’s the responsibility of ISel to generate valid copies and similar should apply to PHIs.

With the latest VS 2019, the error is still the same (MSVC 19.24).

Yep, looks good.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Please upload the diff with full context. This would also need a test.

Formatting

LGTM

LG. It may be worth waiting for other opinions.

Can you check with GNU ld, if --fix-cortex-a53-843419 is ignored when -r is specified?

If @yaxunl has no objections, could someone merge this as I don't have commit access?
Also do we want to also apply this to older versions, since the change is trivial? I confirmed the same problem is in clang 8, 9 and 10 and am certain it is in clang 7, although I didn't test it, because I don't have appropriate device-libs at hand.

LGTM, however it would better if this patch also converted existing code to use this new API.

This was (re)landed as rGff171acf8

Updated patch to correct a comment and fix a typo.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Looks good thanks

Adding the paths for llvm/lld is not needed, because GetProgramPath is actually also searching in $PATH. This means D72903 already is enough to fix my problem.

a4451d88ee456304c26d552749aea6a7f5154bde

In D71932#1827849, @ikudrin wrote:

Thanks for the review!

In D71932#1827601, @dblaikie wrote:

Looks good - future work done as separate patches for separable changes/diagnostics would probably be good (& if you've got some broader goals in terms of what functionality you need/would like in llvm-dwarfdump, an email to llvm-dev explaining the broader goals (do they overlap with James Henderson's goals, improving the line table error handling?) would probably be good - to link to in future reviews, etc)

My main intention is to add support for 64-bit DWARF.

ec9628318d797bfe036aca314d58665dd93b364f

592de0009f6482fbf033ef87d147aab13fcce78c

Oops, this is just the diff from D71681 (which was committed as rG3a05c3969c18 and reverted), but not the whole patch.
I'll reland the whole thing together, but if @yurydelendik or @sunfish have comments I'm happy to hear them.

My initial reaction was that the PHI was invalid, but according to @qcolombet it's ok to have different register banks on the operands

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Thanks for the review!

In D72155#1813778, @jhenderson wrote:

In D72155#1813355, @dblaikie wrote:

Out of curiosity: what's your broader goal with this work? (it'll help understand what's in-scope and out of scope, and better understand the framing when reviewing changes)

There's two parts to this:

1. Making it easier for consumers to continue and try to do something with slightly bad output. One of the problems with using the unrecoverable errors is that it prevents people even trying to iterate over later tables.
2. (this one's not specific to this patch, but some other patches in this area I've been doing are related): we have some local code that uses the DebugInfo library to read a line table. In order for this code to be sound, we need to make sure the line table makes sense. The parser in its current state doesn't pick up on a number of bad situations, so we added some local patches to detect these other errors (local because we didn't have time at that point to try to push them into the open source). We'd like to now get these into the opensource library, to avoid merge conflicts.

In rG03a592f18ba57d52a65e70ad5e1dd709cdcfb71d#885551, @alanphipps wrote:

Hello! Your test specifically looks for "248", which is failing with my downstream embedded ARM compiler (we produce "249"); other similar tests that I've looked at simply ignore this value. Is there a reason why you need to check it specifically?

No, that can be anything.

It looks like the build isn't passing the new flag to cmake. Does the buildbot need to be restarted?

Hello, over the past 2 months since our first draft of Propeller, we've made huge changes/improvements and this describes what happens -http://lists.llvm.org/pipermail/llvm-dev/2020-January/138426.html

Hello! Your test specifically looks for "248", which is failing with my downstream embedded ARM compiler (we produce "249"); other similar tests that I've looked at simply ignore this value. Is there a reason why you need to check it specifically?

• clang-format

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

This patch breaks scan-build-py which parses the output of "-###" to get -cc1 command. There might be other tools with the same problems. Could we either remove (in-process) from CC1Command::Print or add a line break?