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

Permit use of non-literal constants inside inline assembly #12479

Open
bshastry opened this issue Jan 3, 2022 · 14 comments
Open

Permit use of non-literal constants inside inline assembly #12479

bshastry opened this issue Jan 3, 2022 · 14 comments
Labels
must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.

Comments

@bshastry
Copy link
Contributor

bshastry commented Jan 3, 2022

contract C {
  enum E { E1, E2 }
  E constant c = E.E1;
  function f() external {
    assembly {
      let x := c
    }
  }
}

errors on the use of the constant inside inline assembly.

Error: Only direct number constants and references to such constants are supported by inline assembly.

The same happens for constant expressions

contract C {
  bool constant c = (2 > 3 ? true : false);
  function f() external {
    assembly {
      let x := c
    }
  }
}

and constants assigned a constructed value (not literal)

contract C {
  bool constant c = bool(true);
  function f() external {
    assembly {
      let x := c
    }
  }
}
@chriseth
Copy link
Contributor

chriseth commented Jan 3, 2022

Constants are implemented by generating the right-hand side again whenever the constant is referenced. This means you would have to generate solidity code inside assembly code, and I don't think we should do that / it's not worth adding this since there is an easy workaround.

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Apr 21, 2022

I just bumped into this issue while trying to use a user-defined value type in assembly:

type UD60x18 is uint256;

UD60x18 constant SCALE = UD60x18.wrap(1_000000000000000000);

function foo(UD60x18) external pure {
    assembly {
        let remainder := mod(x, SCALE)
    }
}

Getting this error:

Only direct number constants and references to such constants are supported by inline assembly.

I understand the rationale for not doing this for any kind of constant, but maybe we could make an exception for user-defined value types that are uint256 under the hood?

Looks like I can use UD60x18 types in assembly when they are not constants.

@cameel
Copy link
Member

cameel commented Apr 24, 2022

Wouldn't it be feasible to implement by automatically inserting an extra uint local variable before the assembly block and assigning the constant value to it? Then using that variable in the block. The optimizer would just remove the variable most of the time, wouldn't it? So it shouldn't even increase the stack pressure in the optimized code.

I did run into this limitation myself in the past and I have to admit it's really annoying. For example if you have a set of offsets that build on top of each other, you can't just do this (which would be less repetitive and error prone):

uint constant offsetA = 0x20;
uint constant offsetB = offsetA + 5;
uint constant offsetC = offsetB + 0x40;

Instead you have to calculate each value. This is something the compiler should be able to do for you.

@PaulRBerg
Copy link
Contributor

Wouldn't it be feasible to implement by automatically inserting an extra uint local variable before the assembly block and assigning the constant value to it?

Oh yes this is what I am planning on doing .. but I use SCALE in many places. My code will get somewhat verbose because of this limitation related to how constants can be used.

Instead you have to calculate each value. This is something the compiler should be able to do for you.

100%. I really wished I could use non-uint constants in assembly.

@cameel
Copy link
Member

cameel commented Apr 25, 2022

Oh yes this is what I am planning on doing

Sorry, I should have made it clearer that I was asking @chriseth if it wouldn't be viable to implement it this way in the compiler :) I totally think you should not have to this manually, though unfortunately that seems to be the only real workaround right now.

@chriseth
Copy link
Contributor

Yes, we should add the wrap functions to the stuff allowed for constants in inline assembly.

@PaulRBerg
Copy link
Contributor

FWIW, this is the workaround I had to recourse to to make my code compile. I created a copy of each constant by appending a _UINT suffix:

UD60x18 constant HALF_SCALE = UD60x18.wrap(5_00000000000000000);
uint256 constant HALF_SCALE_UINT = 5_00000000000000000;

UD60x18 constant LOG2_E = UD60x18.wrap(1_442695040888963407);
uint256 constant LOG2_E_UINT = 1_442695040888963407;

UD60x18 constant MAX_UD60x18 = UD60x18.wrap(115792089237316195423570985008687907853269984665640564039457_584007913129639935);
uint256 constant MAX_UD60x18_UINT = 115792089237316195423570985008687907853269984665640564039457_584007913129639935;

UD60x18 constant MAX_WHOLE_UD60x18 = UD60x18.wrap(115792089237316195423570985008687907853269984665640564039457_000000000000000000);
uint256 constant MAX_WHOLE_UD60x18_UINT = 115792089237316195423570985008687907853269984665640564039457_000000000000000000;

/// @dev The unit amount which implies how many trailing decimals can be represented.
UD60x18 constant SCALE = UD60x18.wrap(1_000000000000000000);
uint256 constant SCALE_UINT = 1_000000000000000000;

It's not terrible, but not great either. A bit verbose.

@k06a
Copy link

k06a commented Jan 18, 2023

I wish we had this feature to necessarily to have methods and reverts selectors inside assembly. Putting this values into local variables before assembly block limits number of vars we could use inside the block due "Stack too deep" issue. Moreover I wish we could touch immutables inside assembly blocks.

@k06a
Copy link

k06a commented Jan 18, 2023

Yes, we should add the wrap functions to the stuff allowed for constants in inline assembly.

Why not to allow to access every constant? Both literal and non-literal, @chriseth.

@k06a
Copy link

k06a commented Jan 18, 2023

Constants are implemented by generating the right-hand side again whenever the constant is referenced. This means you would have to generate solidity code inside assembly code, and I don't think we should do that / it's not worth adding this since there is an easy workaround.

@chriseth wouldn't it be better to not spend gas on constants in runtime? Why not to recalculate all the constants in the compile time?

@ekpyron
Copy link
Member

ekpyron commented Feb 7, 2023

Constants are implemented by generating the right-hand side again whenever the constant is referenced. This means you would have to generate solidity code inside assembly code, and I don't think we should do that / it's not worth adding this since there is an easy workaround.

@chriseth wouldn't it be better to not spend gas on constants in runtime? Why not to recalculate all the constants in the compile time?

Planned (#13724, #3157) - just not trivial to do safely and in all generality.

@github-actions
Copy link

github-actions bot commented May 9, 2023

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label May 9, 2023
@k06a
Copy link

k06a commented May 9, 2023

image

@ekpyron ekpyron added must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. and removed stale The issue/PR was marked as stale because it has been open for too long. labels May 9, 2023
@ekpyron
Copy link
Member

ekpyron commented May 9, 2023

Haha, I can keep it open :-). But we won't do it directly - it will just fall out of #13724 as a side-effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.
Projects
None yet
Development

No branches or pull requests

7 participants