Skip to content

Commit

Permalink
C-like emitter: Add parenthesis when combining relational and bitwise… (
Browse files Browse the repository at this point in the history 
#6070)

* C-like emitter: Add redundant parentheses in several cases

This is required since the Dawn WGSL compiler requires parentheses even though precedence
rules could resolve order of operations.

This closes #6005.

* Fix tests/metal/byte-address-buffer

The output now includes parentheses around shift expressions appearing as operands in
bitwise expressions, so update the test accordingly.

* format code

---------

Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Yong He <yonghe@outlook.com>
  • Loading branch information
@aleino-nv @slangbot @csyonghe
3 people authored Jan 16, 2025
1 parent ad7d13a commit e771f19
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 148 deletions.
178 changes: 34 additions & 144 deletions source/slang/slang-emit-c-like.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,35 @@ void CLikeSourceEmitter::emitLivenessImpl(IRInst* inst)
// Expressions
//

static bool isBitLogicalOrRelationalOrEquality(EPrecedence prec)
{
switch (prec)
{
case EPrecedence::kEPrecedence_And_Left:
case EPrecedence::kEPrecedence_And_Right:
case EPrecedence::kEPrecedence_BitAnd_Left:
case EPrecedence::kEPrecedence_BitAnd_Right:
case EPrecedence::kEPrecedence_BitOr_Left:
case EPrecedence::kEPrecedence_BitOr_Right:
case EPrecedence::kEPrecedence_BitXor_Left:
case EPrecedence::kEPrecedence_BitXor_Right:
case EPrecedence::kEPrecedence_Or_Left:
case EPrecedence::kEPrecedence_Or_Right:
case EPrecedence::kEPrecedence_Relational_Left:
case EPrecedence::kEPrecedence_Relational_Right:
case EPrecedence::kEPrecedence_Shift_Left:
case EPrecedence::kEPrecedence_Shift_Right:
case EPrecedence::kEPrecedence_Equality_Left:
case EPrecedence::kEPrecedence_Equality_Right:
return true;

default:
break;
}

return false;
}

bool CLikeSourceEmitter::maybeEmitParens(EmitOpInfo& outerPrec, const EmitOpInfo& prec)
{
bool needParens = (prec.leftPrecedence <= outerPrec.leftPrecedence) ||
Expand All @@ -715,152 +744,13 @@ bool CLikeSourceEmitter::maybeEmitParens(EmitOpInfo& outerPrec, const EmitOpInfo
// for common mistakes when parentheses are not used with certain combinations
// of the operations. We emit parentheses to avoid the warnings.
//
// a | b & c => a | (b & c)
if (prec.leftPrecedence == EPrecedence::kEPrecedence_BitAnd_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_BitOr_Right)
{
needParens = true;
}
// a & b | c => (a & b) | c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_BitAnd_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitOr_Left)
{
needParens = true;
}
// a << b + c => a << (b + c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Additive_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_Shift_Right)
{
needParens = true;
}
// a + b << c => (a + b) << c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Additive_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_Shift_Left)
{
needParens = true;
}
// a + b & c => (a + b) & c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Additive_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitAnd_Left)
{
needParens = true;
}
// a ^ b * c => (a ^ b) * c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_BitXor_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_Multiplicative_Left)
{
needParens = true;
}
// a ^ b + c => a ^ (b + c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Additive_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_BitXor_Right)
{
needParens = true;
}
// a + b ^ c => (a + b) ^ c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Additive_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitXor_Left)
{
needParens = true;
}
// a | b + c => a | (b + c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Additive_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_BitOr_Right)
{
needParens = true;
}
// a + b | c => (a + b) | c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Additive_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitOr_Left)
{
needParens = true;
}
// a ^ b * c => a ^ (b * c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Multiplicative_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_BitXor_Right)
{
needParens = true;
}
// a * b ^ c => (a * b) ^ c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Multiplicative_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitXor_Left)
{
needParens = true;
}
// a | b * c => a | (b * c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Multiplicative_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_BitOr_Right)
{
needParens = true;
}
// a * b | c => (a * b) | c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Multiplicative_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitOr_Left)
{
needParens = true;
}
// a & b * c => a & (b * c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Multiplicative_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_BitAnd_Right)
{
needParens = true;
}
// a * b & c => (a * b) & c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Multiplicative_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_BitAnd_Left)
{
needParens = true;
}
// a << b * c => a << (b * c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Multiplicative_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_Shift_Right)
{
needParens = true;
}
// a * b << c => (a * b) << c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Multiplicative_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_Shift_Left)
{
needParens = true;
}
// a != b == c => (a != b) == c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Equality_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_Equality_Left)
{
needParens = true;
}
// a == b < c => a == (b < c)
else if (
prec.leftPrecedence == EPrecedence::kEPrecedence_Relational_Left &&
outerPrec.leftPrecedence == EPrecedence::kEPrecedence_Equality_Right)
{

if (isBitLogicalOrRelationalOrEquality(prec.leftPrecedence) &&
(outerPrec.leftPrecedence > kEPrecedence_Assign_Left))
needParens = true;
}
// a < b == c => (a < b) == c
else if (
prec.rightPrecedence == EPrecedence::kEPrecedence_Relational_Right &&
outerPrec.rightPrecedence == EPrecedence::kEPrecedence_Equality_Left)
{
if (isBitLogicalOrRelationalOrEquality(outerPrec.leftPrecedence) ||
isBitLogicalOrRelationalOrEquality(outerPrec.rightPrecedence))
needParens = true;
}

if (needParens)
{
Expand Down
8 changes: 4 additions & 4 deletions tests/metal/byte-address-buffer.slang
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ struct TestStruct
void main_kernel(uint3 tid: SV_DispatchThreadID)
{
// CHECK: uint [[WORD0:[a-zA-Z0-9_]+]] = as_type<uint>({{.*}}[(int(0))>>2]);
// CHECK: uint8_t [[A:[a-zA-Z0-9_]+]] = uint8_t([[WORD0]] >> 0U & 255U);
// CHECK: uint8_t [[A:[a-zA-Z0-9_]+]] = uint8_t(([[WORD0]] >> 0U) & 255U);
// CHECK: uint [[WORD1:[a-zA-Z0-9_]+]] = as_type<uint>({{.*}}[(int(0))>>2]);
// CHECK: half [[H:[a-zA-Z0-9_]+]] = as_type<half>(ushort([[WORD1]] >> 16U & 65535U));
// CHECK: half [[H:[a-zA-Z0-9_]+]] = as_type<half>(ushort(([[WORD1]] >> 16U) & 65535U));

// CHECK: {{.*}}[(int(128))>>2] = as_type<uint32_t>(({{.*}} & 4294967040U) | uint([[A]]) << 0U);
// CHECK: {{.*}}[(int(128))>>2] = as_type<uint32_t>(({{.*}} & 65535U) | uint(as_type<ushort>([[H]])) << 16U);
// CHECK: {{.*}}[(int(128))>>2] = as_type<uint32_t>(({{.*}} & 4294967040U) | (uint([[A]]) << 0U));
// CHECK: {{.*}}[(int(128))>>2] = as_type<uint32_t>(({{.*}} & 65535U) | (uint(as_type<ushort>([[H]])) << 16U));
buffer.Store(128, buffer.Load<TestStruct>(0));
}

0 comments on commit e771f19

Please sign in to comment.