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.

Sign up for GitHub

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

Jump to bottom

Cherry pick fixes for large signed constant truncation #630

Merged
merged 3 commits into from
Jun 27, 2024
Merged

Cherry pick fixes for large signed constant truncation #630

merged 3 commits into from
Jun 27, 2024

Conversation

vladimirradosavljevic
Copy link
Contributor

No description provided.

@nikic @vladimirradosavljevic
[LoopUnroll] Add test for #80289 (NFC)
dd86bb2
@nikic @vladimirradosavljevic
[LoopUnroll] Fix missing sign extension
007169b 
For integers larger than 64-bit, this would zero-extend a -1
value, instead of sign-extending it.

Fixes llvm/llvm-project#80289.
@nikic @vladimirradosavljevic
Assert correct values in APInt constructor
86e4253 
If the uint64_t constructor is used, assert that the value is
actuall a signed or unsigned N-bit integer depending on whether
the isSigned flag is set.

Currently, we allow values to be silently truncated, which is
a constant source of subtle bugs -- a particularly common mistake
is to create -1 values without setting the isSigned flag, which
will work fine for all common bit widths (<= 64-bit) and miscompile
for larger integers.
@vladimirradosavljevic
Copy link
Contributor Author

vladimirradosavljevic commented Jun 19, 2024 •
edited
Loading

Please note that in the last commit, just parts that fix large signed constant truncation of the upstream PR are taken.

This was referenced Jun 19, 2024
Closed
Closed
Closed
@github-actions GitHub Actions
Copy link

Benchmark results:

╔═╡ Size (-%) ╞════════════════╡ All M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════════════╡ All M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════════════╡ All M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════════════╡ All MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════════════╡ All MzB3 ╞═╣
║ Mean                               -0.000 ║
║ Best                                0.000 ║
║ Worst                              -2.222 ║
║ Total                              -0.000 ║
╠═╡ Ergs (-%) ╞════════════════╡ All MzB3 ╞═╣
║ Mean                               -0.000 ║
║ Best                                0.000 ║
║ Worst                              -1.247 ║
║ Total                              -0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════╡ EVMInterpreter M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                                 NaN ║
╠═╡ Cycles (-%) ╞═══╡ EVMInterpreter M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞═════╡ EVMInterpreter M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs/gas ╞══════╡ EVMInterpreter M3B3 ╞═╣
║ ADD                                66.750 ║
║ MUL                                38.850 ║
║ SUB                                66.750 ║
║ DIV                                47.250 ║
║ SDIV                               64.050 ║
║ MOD                                46.050 ║
║ SMOD                               61.650 ║
║ ADDMOD                             35.656 ║
║ MULMOD                             35.656 ║
║ EXP                                 8.838 ║
║ SIGNEXTEND                         44.850 ║
║ LT                                 70.750 ║
║ GT                                 70.750 ║
║ SLT                                94.750 ║
║ SGT                                92.750 ║
║ EQ                                 70.750 ║
║ ISZERO                             62.417 ║
║ AND                                64.750 ║
║ OR                                 66.750 ║
║ XOR                                66.750 ║
║ NOT                                58.417 ║
║ BYTE                               74.750 ║
║ SHL                                72.750 ║
║ SHR                                70.750 ║
║ SAR                                88.750 ║
║ SGT                                92.750 ║
║ SHA3                               28.647 ║
║ ADDRESS                            90.000 ║
║ BALANCE                            73.585 ║
║ ORIGIN                           1381.562 ║
║ CALLER                             90.000 ║
║ CALLVALUE                          90.000 ║
║ CALLDATALOAD                       60.750 ║
║ CALLDATASIZE                       90.125 ║
║ CALLDATACOPY                       70.200 ║
║ CODESIZE                           90.625 ║
║ CODECOPY                          145.226 ║
║ GASPRICE                         1384.188 ║
║ EXTCODESIZE                         5.107 ║
║ EXTCODECOPY                         5.199 ║
║ RETURNDATASIZE                     85.500 ║
║ RETURNDATACOPY                     54.444 ║
║ EXTCODEHASH                         8.342 ║
║ BLOCKHASH                         243.719 ║
║ COINBASE                         1384.562 ║
║ TIMESTAMP                        1381.562 ║
║ NUMBER                           1381.562 ║
║ PREVRANDAO                       1381.562 ║
║ GASLIMIT                         1387.562 ║
║ CHAINID                          1381.562 ║
║ SELFBALANCE                       653.250 ║
║ BASEFEE                          1378.562 ║
║ POP                                77.625 ║
║ MLOAD                              77.637 ║
║ MSTORE                             81.402 ║
║ MSTORE8                            87.363 ║
║ SLOAD                              27.086 ║
║ SSTORE                              9.059 ║
║ JUMP                               35.778 ║
║ JUMPI                              31.545 ║
║ PC                                 90.500 ║
║ MSIZE                              97.000 ║
║ GAS                                84.500 ║
║ JUMPDEST                          120.000 ║
║ PUSH0                              87.500 ║
║ PUSH1                              66.083 ║
║ PUSH2                              69.500 ║
║ PUSH4                              72.333 ║
║ PUSH5                              73.750 ║
║ PUSH6                              75.167 ║
║ PUSH7                              76.583 ║
║ PUSH8                              78.000 ║
║ PUSH9                              79.417 ║
║ PUSH10                             80.833 ║
║ PUSH11                             82.250 ║
║ PUSH12                             83.667 ║
║ PUSH13                             85.083 ║
║ PUSH14                             86.500 ║
║ PUSH15                             87.917 ║
║ PUSH16                             89.333 ║
║ PUSH17                             90.750 ║
║ PUSH18                             92.167 ║
║ PUSH19                             93.583 ║
║ PUSH20                             95.000 ║
║ PUSH21                             96.417 ║
║ PUSH22                             97.833 ║
║ PUSH23                             99.250 ║
║ PUSH24                            100.667 ║
║ PUSH25                            102.083 ║
║ PUSH26                            103.500 ║
║ PUSH27                            104.917 ║
║ PUSH28                            106.333 ║
║ PUSH29                            107.750 ║
║ PUSH30                            109.167 ║
║ PUSH31                            110.583 ║
║ PUSH32                            110.000 ║
║ DUP1                               60.417 ║
║ DUP2                               64.417 ║
║ DUP3                               64.417 ║
║ DUP4                               64.417 ║
║ DUP5                               64.417 ║
║ DUP6                               64.417 ║
║ DUP7                               64.417 ║
║ DUP8                               64.417 ║
║ DUP9                               62.417 ║
║ DUP10                              64.417 ║
║ DUP11                              64.417 ║
║ DUP12                              64.417 ║
║ DUP13                              64.417 ║
║ DUP14                              64.417 ║
║ DUP15                              64.417 ║
║ DUP16                              64.417 ║
║ SWAP1                              65.083 ║
║ SWAP2                              65.083 ║
║ SWAP3                              65.083 ║
║ SWAP4                              65.083 ║
║ SWAP5                              65.083 ║
║ SWAP6                              65.083 ║
║ SWAP7                              65.083 ║
║ SWAP8                              65.083 ║
║ SWAP9                              65.083 ║
║ SWAP10                             65.083 ║
║ SWAP11                             65.083 ║
║ SWAP12                             65.083 ║
║ SWAP13                             65.083 ║
║ SWAP14                             65.083 ║
║ SWAP15                             65.083 ║
║ SWAP16                             63.083 ║
║ CALL                               63.065 ║
║ STATICCALL                         63.028 ║
║ DELEGATECALL                       62.071 ║
║ CREATE                              5.507 ║
║ CREATE2                             7.770 ║
║ RETURN                              1.000 ║
║ REVERT                              1.000 ║
╠═╡ Ergs/gas (-%) ╞═╡ EVMInterpreter M3B3 ╞═╣
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════╡ EVMInterpreter MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                                 NaN ║
╠═╡ Cycles (-%) ╞═══╡ EVMInterpreter MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞═════╡ EVMInterpreter MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞════════╡ Real life M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞════════╡ Real life MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

@vladimirradosavljevic
Copy link
Contributor Author

Regressions tests are failing, because clang-tidy is complaining about the following:

/__w/era-compiler-llvm/era-compiler-llvm/llvm/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:362:3: error: use auto when initializing with new to avoid duplicating the type name [hicpp-use-auto,-warnings-as-errors]
  362 |   ICmpInst *NewCompare = new ICmpInst(
      |   ^~~~~~~~
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
      |   auto

This is a common code, so I don't think we should address it, since rebase will be harder.

akiramenai
akiramenai approved these changes Jun 27, 2024
View reviewed changes
Copy link
Collaborator

@akiramenai akiramenai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for cherry-picking.

@akiramenai akiramenai merged commit 280f38e into main Jun 27, 2024
11 of 12 checks passed
@akiramenai akiramenai deleted the fix_signed_constant_truncation_for_large_int branch June 27, 2024 09:03
@vladimirradosavljevic vladimirradosavljevic mentioned this pull request Aug 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akiramenai akiramenai akiramenai approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vladimirradosavljevic @akiramenai @nikic