Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress some clang-tidy errors in kpv-cpr-1568 #446

Open
wants to merge 658 commits into
base: kpv-cpr-1568-adding-support-of-evm-arch-to-lld-linker
Choose a base branch
from

Conversation

FlashSheridan
Copy link
Contributor

@FlashSheridan FlashSheridan commented Mar 21, 2024

Purpose

Suppress some clang-tidy errors in branch kpv-cpr-1568-adding-support-of-evm-arch-to-lld-linker; I’m proposing to add these changes to that branch while it‘s in development. (Some of the debugging output in compiler-integration-tests.yml may not be suitable for main.)

Ticket Number

CPR-1600

Implementation

  • Pass the googlemock and googletest paths on the clang-tidy-diff_Zegar.py command-line for otherwise apparently unsuppressible failures.
  • Work around a clang-tidy bug in lld/.clang-tidy by having one positive check (clang-analyzer-osx.cocoa.MissingSuperCall, which is unlikely to conflict with anything.)
  • Suppress all clang-tidy warnings in lld/unittests.
  • Manually apply upstream updates to clang-tidy-diff_Zegar.py, and silence some Pylint (and probably runtime) warnings.
  • Revert to specifying clang-tidy-15 r/t generic clang-tidy.

This does not fix the unsuppressible breakage from “llvm/llvm/tools/llvm-driver/llvm-driver.cpp:20:10: error: 'LLVMDriverTools.def' file not found [clang-diagnostic-error]”, which is in Chris Bieneman’s code, not ours, and which I don’t understand the CMake setup well enough to address.

There are remaining warnings in, for instance, lld/MinGW/Driver.cpp, many of them due to Alexandre Ganea; I hope that the WarningsAsErrors: '' in lld/.clang-tidy will prevent these from breaking the GitHub Action once the [clang-diagnostic-error]s are fixed. (I agree with that we should ignore them rather than fix them.) If not, or if there‘s too much noise, reverting e91ad1f (“Remove the universal suppression from lld, now that it's in lld/unittests”) may help.

akiramenai and others added 30 commits December 1, 2022 15:48
Prevent DAGCombiner from narrowing loads via TLI::shouldReduceLoadWidth,
inhibit [zs]ext(load) -> [zs]extload transformation.
Fat pointer type is to used to distinguish with regular i256 type,
which does not require special instruction handling to maintain its
tagged status within VM.

Pointers to generic address space will be interpreted as fat pointers
in the backend. It will be assigned a physical register which could be
overlapped with i256 -- this is conformant to fat pointer semantics.
Co-authored-by: Alex Z <a.zarudnyy@matterlabs.dev>
Farcalls now return resulting fat pointer in R1 and success flag in R2.
With the help of free flag setting, we could try to fold
a comparison with 0 (part of select or branching) with previous instruction.
Also make the soft toolchain requirements hard. This allows
us to use C++17 features in LLVM now.

If we find patterns with C++17 that improve readability
it should be recommended in the coding standards.

Reviewed By: jhenderson, cor3ntin, MaskRay

Differential Revision: https://reviews.llvm.org/D130689

Conflicts:
	llvm/docs/ReleaseNotes.rst
Our previous value 10.9 causes an error with our build setup:
>    'shared_mutex' is unavailable: introduced in macOS 10.12
There are ways around this; see https://reviews.llvm.org/D66313,
but testing MacOS 11, 12, and 13 seems reasonable.
Add ourselves to Windows as not supporting file permission error-checks,
though it’s actually that our CI runs too permissively.
We should not allow ISEL failure because of cannot select. For all unsupported
types they must all be marked as `Expand` or explicitly fail with a reason.

* expand remaining ISEL nodes that are not supported on our target
* support of 256-bit CTPOP expansion by reusing 128-bit parallel counting algorithm.
* custom lower CTPOP and BSWAP for 256bit
Remove unsupported from lit config for generic codegen tests and handle
every single case on it's own:

* Relax requirement on global constants to be i256.
* Wrap block address and external symbol to materialize it in a register
  before using.
* Wrap global address in calling conventions.
* Disable bit tests clustering lowering for switches.
* Fix handling of negative wrt to FrameIndex offsets.
* Mark tests that have floats, vectors, inline asm, EH but cleanups as
  UNSUPPORTED.
* Classify and mark as TODO still unhandled failures.
We do not distinguish conditional jumps and unconditional jumps, but in fact conditional jumps are not barriers.

This patch separates conditional jumps and unconditional jumps and update analyzeBranch to properly return analyzed results.
* in our architecture, CALLSTACK take CImm operands instead of imm
* CALLs should not be modeled as terminators
* also update the test file to enable testing these changes
Because fat pointers are not numeric, LLVM doesn't know how to assert
alignment, so trust the FE on that respect.
When a global array is accessed by a constant index LLVM refers to it as
symbol+offset where offset is in bytes. The patch converts offset to
cells.
The transformation works however only for global variable arrays (i.e.
stack address space). Our handling of constant arrays (code address
space) require isel pattern rework (CPR-940). So the patch disables
SimplifyCFG's logic that convert switches to const arrays since it causes
code generation issues.
Which will cause `--verify-regalloc` to fail due to incompatible
type of operand. Use `SyncVM::R0` instead of `0`.
This is needed to fix the `--verify-twice` option, as it will
run the compilation pipeline twice.

The problem was that we did not reset cached constant pool entries
after finishing emitting.
Manually squash single file, as git says CONFLICT (content): Merge conflict in .github/workflows/compiler-integration-tests.yml
error: could not apply 9b2e437
* Export commit ID getting API as LLVMPrintCommitIDTo.
* Print commit id in LLVM tool --version message.
INVOKE is eventually converted to a near call, which should not be
a terminator by itself. The throwing and error handling is already
done in other parts.

This patch fixes the following test:

```
llc --verify-regalloc  --verify-machineinstrs llvm/test/CodeGen/SyncVM/cxa_throw.ll
```
PavelKopyl and others added 17 commits March 14, 2024 14:38
before trying fixes for some Pylint warnings.
warnings, which seemed to silence similar warnings during
execution, and might conceivably help with false positives
and failed suppressions.
(Didn’t work completely locally, e.g. Driver.cpp:2078:29 clang-analyzer-core.NullDereference)
command-line for otherwise apparently unsuppressible failures.
Copy link

Benchmark results:

╔═╡ Size (-%) ╞════════════════╡ All M3B3 ╞═╗
║ Mean                                0.174 ║
║ Best                               13.333 ║
║ Worst                             -39.344 ║
║ Total                               0.055 ║
╠═╡ Cycles (-%) ╞══════════════╡ All M3B3 ╞═╣
║ Mean                               -0.136 ║
║ Best                                8.163 ║
║ Worst                             -36.407 ║
║ Total                              -0.047 ║
╠═╡ Ergs (-%) ╞════════════════╡ All M3B3 ╞═╣
║ Mean                               -0.085 ║
║ Best                                5.128 ║
║ Worst                             -29.014 ║
║ Total                              -0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════════════╡ All MzB3 ╞═╗
║ Mean                               -0.047 ║
║ Best                                6.897 ║
║ Worst                             -28.571 ║
║ Total                              -0.110 ║
╠═╡ Cycles (-%) ╞══════════════╡ All MzB3 ╞═╣
║ Mean                               -0.065 ║
║ Best                               11.261 ║
║ Worst                             -26.971 ║
║ Total                              -0.034 ║
╠═╡ Ergs (-%) ╞════════════════╡ All MzB3 ╞═╣
║ Mean                               -0.033 ║
║ Best                               11.087 ║
║ Worst                             -21.643 ║
║ Total                              -0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles M3B3 ╞═╗
║ Mean                                0.342 ║
║ Best                                0.966 ║
║ Worst                               0.000 ║
║ Total                               0.449 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles M3B3 ╞═╣
║ Mean                               -0.409 ║
║ Best                                0.627 ║
║ Worst                             -15.464 ║
║ Total                              -0.022 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles M3B3 ╞═╣
║ Mean                               -0.361 ║
║ Best                                0.573 ║
║ Worst                             -12.040 ║
║ Total                              -0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles MzB3 ╞═╗
║ Mean                               -0.099 ║
║ Best                                0.000 ║
║ Worst                              -1.117 ║
║ Total                              -0.068 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles MzB3 ╞═╣
║ Mean                               -0.174 ║
║ Best                                5.556 ║
║ Worst                             -13.793 ║
║ Total                              -0.063 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles MzB3 ╞═╣
║ Mean                               -0.188 ║
║ Best                                3.175 ║
║ Worst                             -10.805 ║
║ Total                              -0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life M3B3 ╞═╗
║ Mean                               -0.399 ║
║ Best                                2.372 ║
║ Worst                              -6.351 ║
║ Total                              -0.517 ║
╠═╡ Cycles (-%) ╞════════╡ Real life M3B3 ╞═╣
║ Mean                               -0.191 ║
║ Best                                0.373 ║
║ Worst                              -3.040 ║
║ Total                              -0.173 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life M3B3 ╞═╣
║ Mean                               -0.016 ║
║ Best                                0.880 ║
║ Worst                              -0.807 ║
║ Total                              -0.003 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life MzB3 ╞═╗
║ Mean                               -0.108 ║
║ Best                                0.830 ║
║ Worst                              -1.741 ║
║ Total                              -0.091 ║
╠═╡ Cycles (-%) ╞════════╡ Real life MzB3 ╞═╣
║ Mean                               -0.192 ║
║ Best                                1.277 ║
║ Worst                              -2.932 ║
║ Total                              -0.353 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life MzB3 ╞═╣
║ Mean                               -0.021 ║
║ Best                                0.906 ║
║ Worst                              -0.960 ║
║ Total                              -0.006 ║
╚═══════════════════════════════════════════╝

@PavelKopyl PavelKopyl force-pushed the kpv-cpr-1568-adding-support-of-evm-arch-to-lld-linker branch from e55e78d to d237304 Compare March 25, 2024 16:47
@PavelKopyl PavelKopyl force-pushed the kpv-cpr-1568-adding-support-of-evm-arch-to-lld-linker branch 2 times, most recently from 31248da to c4b7b65 Compare April 19, 2024 12:46
@hedgar2017 hedgar2017 force-pushed the kpv-cpr-1568-adding-support-of-evm-arch-to-lld-linker branch from c4b7b65 to 2010039 Compare April 21, 2024 08:16
@PavelKopyl PavelKopyl force-pushed the kpv-cpr-1568-adding-support-of-evm-arch-to-lld-linker branch 2 times, most recently from c879900 to f443a5f Compare April 26, 2024 07:37
@PavelKopyl PavelKopyl force-pushed the kpv-cpr-1568-adding-support-of-evm-arch-to-lld-linker branch 2 times, most recently from 760de6c to 7c76b3e Compare May 13, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet