diff --git a/.github/workflows/buildAndTest.yml b/.github/workflows/buildAndTest.yml index 42483ae6e20c..797449e4511f 100644 --- a/.github/workflows/buildAndTest.yml +++ b/.github/workflows/buildAndTest.yml @@ -14,16 +14,20 @@ jobs: name: Sanity Check runs-on: ubuntu-latest container: - image: ghcr.io/circt/images/circt-ci-build:20220216182244 + image: ghcr.io/circt/images/circt-ci-build:20240213211952 steps: # Clone the CIRCT repo and its submodules. Do shallow clone to save clone # time. - name: Get CIRCT - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: fetch-depth: 2 submodules: "false" + - name: Set git safe + run: | + git config --global --add safe.directory $PWD + # -------- # Lint the CIRCT C++ code. # ------- @@ -93,7 +97,7 @@ jobs: # Upload the format patches to an artifact (zip'd) associated # with the workflow run. Only run this on a failure. - name: Upload format patches - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 continue-on-error: true if: ${{ failure() }} with: @@ -119,12 +123,12 @@ jobs: # Configure CIRCT using LLVM's build system ("Unified" build). We do not actually build this configuration since it isn't as easy to cache LLVM artifacts in this mode. configure-circt-unified: name: Configure Unified Build - runs-on: ubuntu-18.04 + runs-on: ubuntu-22.04 steps: # Clone the CIRCT repo and its submodules. Do shallow clone to save clone # time. - name: Get CIRCT - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: fetch-depth: 2 submodules: "true" @@ -150,7 +154,7 @@ jobs: needs: sanity-check runs-on: ubuntu-latest container: - image: ghcr.io/circt/images/circt-ci-build:20220216182244 + image: ghcr.io/circt/images/circt-ci-build:20240213211952 strategy: matrix: compiler: @@ -175,7 +179,7 @@ jobs: # Clone the CIRCT repo and its submodules. Do shallow clone to save clone # time. - name: Get CIRCT - uses: actions/checkout@v2 + uses: actions/checkout@v3 with: fetch-depth: 2 submodules: "true" @@ -196,7 +200,7 @@ jobs: # Try to fetch LLVM from the cache. - name: Cache LLVM id: cache-llvm - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: | llvm/build @@ -292,7 +296,7 @@ jobs: # Upload the tidy patches to an artifact (zip'd) associated # with the workflow run. Only run this on a failure. - name: Upload tidy patches - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 continue-on-error: true if: ${{ failure() }} with: diff --git a/lib/Conversion/ExportVerilog/ExportVerilog.cpp b/lib/Conversion/ExportVerilog/ExportVerilog.cpp index 722b0bd888d6..2cf15a2d12ac 100644 --- a/lib/Conversion/ExportVerilog/ExportVerilog.cpp +++ b/lib/Conversion/ExportVerilog/ExportVerilog.cpp @@ -1755,22 +1755,6 @@ SubExprInfo ExprEmitter::emitBinary(Operation *op, VerilogPrecedence prec, if (!isa(op)) rhsPrec = VerilogPrecedence(prec - 1); - // Introduce extra parentheses to specific patterns of expressions. - // If op is "AndOp", and rhs is Reduction And, the output is like `a & &b`. - // This is syntactically valid but some tool produces LINT warnings. Also it - // would be confusing for users to read such expressions. - bool emitRhsParentheses = false; - if (auto rhsICmp = op->getOperand(1).getDefiningOp()) { - if ((rhsICmp.isEqualAllOnes() && isa(op)) || - (rhsICmp.isNotEqualZero() && isa(op))) { - if (isExpressionEmittedInline(rhsICmp)) { - os << '('; - emitRhsParentheses = true; - rhsPrec = LowestPrecedence; - } - } - } - // If the RHS operand has self-determined width and always treated as // unsigned, inform emitSubExpr of this. This is true for the shift amount in // a shift operation. @@ -1782,8 +1766,6 @@ SubExprInfo ExprEmitter::emitBinary(Operation *op, VerilogPrecedence prec, auto rhsInfo = emitSubExpr(op->getOperand(1), rhsPrec, operandSignReq, rhsIsUnsignedValueWithSelfDeterminedWidth); - if (emitRhsParentheses) - os << ')'; // SystemVerilog 11.8.1 says that the result of a binary expression is signed // only if both operands are signed. @@ -1804,7 +1786,11 @@ SubExprInfo ExprEmitter::emitUnary(Operation *op, const char *syntax, bool resultAlwaysUnsigned) { os << syntax; auto signedness = emitSubExpr(op->getOperand(0), Selection).signedness; - return {Unary, resultAlwaysUnsigned ? IsUnsigned : signedness}; + // For reduction operators "&" and "|", make precedence lowest to avoid + // emitting an expression like `a & &b`, which is syntactically valid but some + // tools produce LINT warnings. + return {isa(op) ? LowestPrecedence : Unary, + resultAlwaysUnsigned ? IsUnsigned : signedness}; } /// This function split the output buffer into multiple lines if the emitted diff --git a/test/Conversion/ExportVerilog/hw-dialect.mlir b/test/Conversion/ExportVerilog/hw-dialect.mlir index 4695e866f4aa..4b07b2b559ba 100644 --- a/test/Conversion/ExportVerilog/hw-dialect.mlir +++ b/test/Conversion/ExportVerilog/hw-dialect.mlir @@ -992,13 +992,17 @@ hw.module @addParenthesesToSuccessiveOperators(%a: i4, %b: i1, %c: i4) -> (o1:i1 %zero4 = hw.constant 0 : i4 // CHECK: wire [[GEN:.+]] = &c; - %0 = comb.icmp eq %a, %one4 : i4 - %and = comb.and %b, %0 : i1 - // CHECK-NEXT: assign o1 = b & (&a); - - %1 = comb.icmp ne %a, %zero4 : i4 - %or = comb.or %b, %1 : i1 - // CHECK-NEXT: assign o2 = b | (|a); + %and1 = comb.icmp eq %a, %one4 : i4 + %and2 = comb.icmp eq %a, %one4 : i4 + %and3 = comb.icmp eq %a, %one4 : i4 + %and = comb.and %and1, %b, %and2, %and3 : i1 + // CHECK-NEXT: assign o1 = (&a) & b & (&a) & (&a); + + %or1 = comb.icmp ne %a, %zero4 : i4 + %or2 = comb.icmp ne %a, %zero4 : i4 + %or3 = comb.icmp ne %a, %zero4 : i4 + %or = comb.or %or1, %b, %or2, %or3 : i1 + // CHECK-NEXT: assign o2 = (|a) | b | (|a) | (|a); %3 = comb.icmp eq %c, %one4 : i4 %multiuse = comb.and %3, %3 : i1