Page MenuHomePhabricator

[AMDGPU][MC] Enabled generic constant expressions as operands

Authored by dp on Apr 16 2019, 4:17 AM.



See bug 40873:

Before this change assembler required constant expressions to start with a number, for example:

x = 1
v_or3_b32 v1, v2, v3, 1+x         // ok
v_or3_b32 v1, v2, v3, x+1         // error
v_ceil_f32 v1, abs(1+x)           // ok
v_ceil_f32 v1, abs(x+1)           // error

This fix enables to use arbitrary constant expressions with generic imm operands and operands of VOP modifiers like abs(...) and neg(...):

x = 1
v_or3_b32 v1, v2, v3, 1+x         // ok
v_or3_b32 v1, v2, v3, x+1         // ok
v_ceil_f32 v1, abs(1+x)           // ok
v_ceil_f32 v1, abs(x+1)           // ok

Some syntactic sugar constructs like hwreg(...) still expect constant expressions to start with a number; this will be fixed separately. See bug 40820:

Diff Detail


Event Timeline

dp created this revision.Apr 16 2019, 4:17 AM
dp added a reviewer: vpykhtin.Apr 23 2019, 2:19 AM
artem.tamazov accepted this revision.Apr 26 2019, 5:14 AM

LGTM. Recommendation: it would be nice if error message will be more descriptive (where is does not require too much work)

9 ↗(On Diff #195347)

Can we be more descriptive in the message? like "error: Floating-point expressions are not supported"?

This revision is now accepted and ready to land.Apr 26 2019, 5:14 AM
dp marked an inline comment as done.Apr 26 2019, 6:11 AM
dp added inline comments.
9 ↗(On Diff #195347)

In theory we can improve error message by looking at the next token. If the token is a binary operator like '*' the whole thing looks like a floating-point expression which we do not support.

However, there are some issues with this approach:

  1. We have a lot of binary operators to check - see
  2. Operands do not have to be separated with commas, spaces are allowed as well. As a result, we may be unable to distinguish an expression from two separate operands, for example:
s_sub_u32 s0, -1.0 + 1     // assembled ok!

This code is equivalent to the following:

s_sub_u32 s0, -1.0, + 1
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2019, 6:16 AM