Skip to content

[NFC][LLVM][TableGen] Use decodeULEB128 for OPC_SoftFail emission #136220

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

Merged
merged 1 commit into from
Apr 18, 2025

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Apr 17, 2025

  • Use decodeULEB128 to decode +ve/-ve mask in OPC_SoftFail case.
  • Use current I/E iterators as inputs to decodeULEB128.

@jurahul jurahul force-pushed the decoder_emitter_use_decodeuleb_softfail branch from fbee99a to e1820b7 Compare April 17, 2025 23:34
- Use `decodeULEB128` to decode +ve/-ve mask in OPC_SoftFail case.
- Use current I/E iterators as inputs to `decodeULEB128`.
@jurahul jurahul force-pushed the decoder_emitter_use_decodeuleb_softfail branch from e1820b7 to c446bc2 Compare April 17, 2025 23:53
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jurahul jurahul marked this pull request as ready for review April 18, 2025 00:33
@llvmbot
Copy link
Member

llvmbot commented Apr 18, 2025

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes
  • Use decodeULEB128 to decode +ve/-ve mask in OPC_SoftFail case.
  • Use current I/E iterators as inputs to decodeULEB128.

Full diff: https://github.com/llvm/llvm-project/pull/136220.diff

1 Files Affected:

  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+18-32)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 9c6015cc24576..0f775673e599f 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -849,8 +849,7 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
 
       // ULEB128 encoded start value.
       const char *ErrMsg = nullptr;
-      unsigned Start = decodeULEB128(Table.data() + Pos + 1, nullptr,
-                                     Table.data() + Table.size(), &ErrMsg);
+      unsigned Start = decodeULEB128(&*I, nullptr, &*E, &ErrMsg);
       assert(ErrMsg == nullptr && "ULEB128 value too large!");
       emitULEB128(I, OS);
 
@@ -904,8 +903,7 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       ++I;
       // Decode the Opcode value.
       const char *ErrMsg = nullptr;
-      unsigned Opc = decodeULEB128(Table.data() + Pos + 1, nullptr,
-                                   Table.data() + Table.size(), &ErrMsg);
+      unsigned Opc = decodeULEB128(&*I, nullptr, &*E, &ErrMsg);
       assert(ErrMsg == nullptr && "ULEB128 value too large!");
 
       OS << Indent << "MCD::OPC_" << (IsTry ? "Try" : "") << "Decode, ";
@@ -934,34 +932,22 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
     }
     case MCD::OPC_SoftFail: {
       ++I;
-      OS << Indent << "MCD::OPC_SoftFail";
-      // Positive mask
-      uint64_t Value = 0;
-      unsigned Shift = 0;
-      do {
-        OS << ", " << (unsigned)*I;
-        Value += ((uint64_t)(*I & 0x7f)) << Shift;
-        Shift += 7;
-      } while (*I++ >= 128);
-      if (Value > 127) {
-        OS << " /* 0x";
-        OS.write_hex(Value);
-        OS << " */";
-      }
-      // Negative mask
-      Value = 0;
-      Shift = 0;
-      do {
-        OS << ", " << (unsigned)*I;
-        Value += ((uint64_t)(*I & 0x7f)) << Shift;
-        Shift += 7;
-      } while (*I++ >= 128);
-      if (Value > 127) {
-        OS << " /* 0x";
-        OS.write_hex(Value);
-        OS << " */";
-      }
-      OS << ",\n";
+      OS << Indent << "MCD::OPC_SoftFail, ";
+      // Decode the positive mask.
+      const char *ErrMsg = nullptr;
+      uint64_t PositiveMask = decodeULEB128(&*I, nullptr, &*E, &ErrMsg);
+      assert(ErrMsg == nullptr && "ULEB128 value too large!");
+      emitULEB128(I, OS);
+
+      // Decode the negative mask.
+      uint64_t NegativeMask = decodeULEB128(&*I, nullptr, &*E, &ErrMsg);
+      assert(ErrMsg == nullptr && "ULEB128 value too large!");
+      emitULEB128(I, OS);
+      OS << "// +ve mask: 0x";
+      OS.write_hex(PositiveMask);
+      OS << ", -ve mask: 0x";
+      OS.write_hex(NegativeMask);
+      OS << '\n';
       break;
     }
     case MCD::OPC_Fail: {

@jurahul
Copy link
Contributor Author

jurahul commented Apr 18, 2025

Thanks.

@jurahul jurahul merged commit 3ed8363 into llvm:main Apr 18, 2025
14 checks passed
@jurahul jurahul deleted the decoder_emitter_use_decodeuleb_softfail branch April 18, 2025 12:12
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 18, 2025

LLVM Buildbot has detected a new failure on builder flang-x86_64-windows running on minipc-ryzen-win while building llvm at step 8 "test-build-unified-tree-check-flang-rt".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/166/builds/1191

Here is the relevant piece of the build log for the reference
Step 8 (test-build-unified-tree-check-flang-rt) failure: test (failure)
******************** TEST 'flang-rt-Unit :: Runtime/./RuntimeTests.exe/2/8' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:C:\buildbot\flang-x86_64-windows\build\runtimes\runtimes-bins\flang-rt\unittests\Runtime\.\RuntimeTests.exe-flang-rt-Unit-4796-2-8.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=8 GTEST_SHARD_INDEX=2 C:\buildbot\flang-x86_64-windows\build\runtimes\runtimes-bins\flang-rt\unittests\Runtime\.\RuntimeTests.exe
--

Note: This is test shard 3 of 8.

[==========] Running 28 tests from 24 test suites.

[----------] Global test environment set-up.

[----------] 1 test from AllocatableTest

[ RUN      ] AllocatableTest.AllocateSourceZeroSize

[       OK ] AllocatableTest.AllocateSourceZeroSize (0 ms)

[----------] 1 test from AllocatableTest (0 ms total)



[----------] 1 test from AdjustLRTests/0, where TypeParam = <type>

[ RUN      ] AdjustLRTests/0.AdjustL

[       OK ] AdjustLRTests/0.AdjustL (0 ms)

[----------] 1 test from AdjustLRTests/0 (0 ms total)



[----------] 1 test from CharacterComparisonTests/2, where TypeParam = <type>

[ RUN      ] CharacterComparisonTests/2.CompareCharacters

[       OK ] CharacterComparisonTests/2.CompareCharacters (0 ms)

[----------] 1 test from CharacterComparisonTests/2 (0 ms total)



[----------] 1 test from ExtremumTests/2, where TypeParam = <type>

[ RUN      ] ExtremumTests/2.MaxTests

[       OK ] ExtremumTests/2.MaxTests (0 ms)

[----------] 1 test from ExtremumTests/2 (0 ms total)

...

@jurahul
Copy link
Contributor Author

jurahul commented Apr 18, 2025

This seems to have causes a build failure on some Windows systems, I am fixing it here:

#136310

jurahul added a commit that referenced this pull request Apr 18, 2025
- Avoid dereferencing the end() iterator to get the end pointer, instead
calculate it explicitly
- Fixes a regression introduced in
#136220.
- The windows build failure shows the following call stack:

```
 | Exception Code: 0x80000003
 |  #0 0x00007ff74bc05897 std::_Vector_const_iterator<class std::_Vector_val<struct std::_Simple_types<unsigned char>>>::operator*(void) const C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\vector:52:0
 |  #1 0x00007ff74bbd3d64 `anonymous namespace'::DecoderEmitter::emitTable D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\llvm\utils\TableGen\DecoderEmitter.cpp:852:0
```
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Apr 18, 2025
…36310)

- Avoid dereferencing the end() iterator to get the end pointer, instead
calculate it explicitly
- Fixes a regression introduced in
llvm/llvm-project#136220.
- The windows build failure shows the following call stack:

```
 | Exception Code: 0x80000003
 |  #0 0x00007ff74bc05897 std::_Vector_const_iterator<class std::_Vector_val<struct std::_Simple_types<unsigned char>>>::operator*(void) const C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.37.32822\include\vector:52:0
 |  #1 0x00007ff74bbd3d64 `anonymous namespace'::DecoderEmitter::emitTable D:\buildbot\llvm-worker\clang-cmake-x86_64-avx512-win\llvm\llvm\utils\TableGen\DecoderEmitter.cpp:852:0
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants