Skip to content

Conversation

@redstar
Copy link
Member

@redstar redstar commented Jan 5, 2026

Using a raw_svector_ostream object is not necessary, because this is hidden in the conversion function. In addition, there is no need to reason about a zero termination of the string. Declaring the ascii and ebcdic version of the string variables at the same time makes sure that both strings are allocated with the same size.

Using a `raw_svector_ostream` object is not necessary, because this is
hidden in the conversion function. In addition, there is no need to
reason about a zero termination of the string. Declaring the ascii and
ebcdic version of the string variables at the same time makes sure that
both strings are allocated with the same size.
@redstar redstar requested a review from uweigand January 5, 2026 23:04
@redstar redstar self-assigned this Jan 5, 2026
@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2026

@llvm/pr-subscribers-backend-systemz

Author: Kai Nacke (redstar)

Changes

Using a raw_svector_ostream object is not necessary, because this is hidden in the conversion function. In addition, there is no need to reason about a zero termination of the string. Declaring the ascii and ebcdic version of the string variables at the same time makes sure that both strings are allocated with the same size.


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

1 Files Affected:

  • (modified) llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp (+9-15)
diff --git a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
index 444bc578a3fab..332a38281eab0 100644
--- a/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZAsmPrinter.cpp
@@ -1602,25 +1602,19 @@ void SystemZAsmPrinter::emitPPA2(Module &M) {
   MCSymbol *DateVersionSym = OutContext.createTempSymbol("DVS", false);
 
   std::time_t Time = getTranslationTime(M);
-  SmallString<15> CompilationTime; // 14 + null
-  raw_svector_ostream O(CompilationTime);
-  O << formatv("{0:%Y%m%d%H%M%S}", llvm::sys::toUtcTime(Time));
+  SmallString<14> CompilationTimeEBCDIC, CompilationTime;
+  CompilationTime = formatv("{0:%Y%m%d%H%M%S}", llvm::sys::toUtcTime(Time));
 
   uint32_t ProductVersion = getProductVersion(M),
            ProductRelease = getProductRelease(M),
            ProductPatch = getProductPatch(M);
 
-  SmallString<7> Version; // 6 + null
-  raw_svector_ostream ostr(Version);
-  ostr << formatv("{0,0-2:d}{1,0-2:d}{2,0-2:d}", ProductVersion, ProductRelease,
-                  ProductPatch);
+  SmallString<6> VersionEBCDIC, Version;
+  Version = formatv("{0,0-2:d}{1,0-2:d}{2,0-2:d}", ProductVersion,
+                    ProductRelease, ProductPatch);
 
-  // Drop 0 during conversion.
-  SmallString<sizeof(CompilationTime) - 1> CompilationTimeStr;
-  SmallString<sizeof(Version) - 1> VersionStr;
-
-  ConverterEBCDIC::convertToEBCDIC(CompilationTime, CompilationTimeStr);
-  ConverterEBCDIC::convertToEBCDIC(Version, VersionStr);
+  ConverterEBCDIC::convertToEBCDIC(CompilationTime, CompilationTimeEBCDIC);
+  ConverterEBCDIC::convertToEBCDIC(Version, VersionEBCDIC);
 
   enum class PPA2MemberId : uint8_t {
     // See z/OS Language Environment Vendor Interfaces v2r5, p.23, for
@@ -1689,8 +1683,8 @@ void SystemZAsmPrinter::emitPPA2(Module &M) {
 
   // Emit date and version section.
   OutStreamer->emitLabel(DateVersionSym);
-  OutStreamer->emitBytes(CompilationTimeStr.str());
-  OutStreamer->emitBytes(VersionStr.str());
+  OutStreamer->emitBytes(CompilationTimeEBCDIC.str());
+  OutStreamer->emitBytes(VersionEBCDIC.str());
 
   OutStreamer->emitInt16(0x0000); // Service level string length.
 

Copy link
Contributor

@perry-ca perry-ca left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@uweigand uweigand left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@redstar redstar merged commit 5f590ed into main Jan 8, 2026
12 checks passed
@redstar redstar deleted the users/redstar/betteruseformatv branch January 8, 2026 14:34
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 8, 2026

LLVM Buildbot has detected a new failure on builder ppc64le-mlir-rhel-clang running on ppc64le-mlir-rhel-test while building llvm at step 6 "test-build-check-mlir-build-only-check-mlir".

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

Here is the relevant piece of the build log for the reference
Step 6 (test-build-check-mlir-build-only-check-mlir) failure: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
...
PASS: MLIR :: Transforms/scf-loop-unroll.mlir (3787 of 3798)
PASS: MLIR :: Dialect/Vector/vector-warp-distribute.mlir (3788 of 3798)
PASS: MLIR :: Transforms/test-operation-folder.mlir (3789 of 3798)
PASS: MLIR :: mlir-tblgen/op-error.td (3790 of 3798)
PASS: MLIR :: mlir-runner/verify-entry-point.mlir (3791 of 3798)
PASS: MLIR :: Pass/pipeline-options-parsing.mlir (3792 of 3798)
PASS: MLIR :: mlir-tblgen/llvm-intrinsics.td (3793 of 3798)
PASS: MLIR :: mlir-tblgen/rewriter-errors.td (3794 of 3798)
PASS: MLIR :: mlir-tblgen/cpp-class-comments.td (3795 of 3798)
PASS: MLIR :: mlir-runner/simple.mlir (3796 of 3798)
command timed out: 1200 seconds without output running [b'ninja', b'check-mlir'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1523.344413

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants