prb-math icon indicating copy to clipboard operation
prb-math copied to clipboard

Use optimal comparison operator

Open kadenzipfel opened this issue 3 years ago • 1 comments

for every uint > 0 check, replace with uint != 0 to save about 44 gas per call see: https://github.com/KadenZipfel/gas-optimizations/blob/main/gas-saving-patterns/optimal-comparison-operator.md

kadenzipfel avatar Jun 22 '22 16:06 kadenzipfel

Hey @kadenzipfel, thanks so much for opening this PR.

First off, great work with https://github.com/kadenzipfel/gas-optimizations! Though if I may suggest something, it may be useful to add a Solidity pragma.

Then, I will have to double check that using > 0 instead of != 0 would lead to an optimization in the latest version of Solidity. I recently had a conversation about this very topic on Twitter, and it turns that by adding the --via-ir flag when compiling, you get the same reduced gas cost even with > 0.

This tweet is also related.

PaulRBerg avatar Jul 03 '22 08:07 PaulRBerg

Hi @kadenzipfel, sorry for the long review time.

I am happy to accept the changes introduced by this PR, because it is not a given that everybody will use --via-ir. It's not as if PRBMath is pre-compiled and then exported to the world - PRBMath users compile the library source code by themselves.

The trouble is, quite a few things have changed, and there are lots of conflicts when trying to merge this onto staging.

Could I kindly ask you to re-apply your changes onto the tip of the staging branch, and git push with force to your branch?

Thank you! Also happy to do the above myself, but then you wouldn't show up as a contributor.

PaulRBerg avatar Nov 25 '22 19:11 PaulRBerg

I managed to re-apply your changes on top of the staging branch myself, while preserving your git authorship.

However, after a little bit of research and benchmarking on Remix, I realized that the != operator is not more gas efficient than >. I am unsure what you led you to conclude that it is, but I documented my findings in this issue in your repo: https://github.com/kadenzipfel/gas-optimizations/issues/23.

I will close this PR until proven otherwise.

PaulRBerg avatar Nov 28 '22 12:11 PaulRBerg