Skip to content

[SystemZ][z/OS] Refactor AutoConvert.h to remove large MVS guard #143174

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 3 commits into from
Jun 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions llvm/include/llvm/Support/AutoConvert.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#ifdef __MVS__
#include <_Ccsid.h>
#endif
#ifdef __cplusplus
#include "llvm/Support/ErrorOr.h"
#include <system_error>
Expand All @@ -28,16 +29,58 @@
#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */

int enablezOSAutoConversion(int FD);
Copy link
Contributor

Choose a reason for hiding this comment

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

How are these C variations of the functions handled? Do we declare macros for enableAutoConversion() if not c++ and expand that to enablezOSAutoConversion() for MVS? And how does this integrate with the c++ inline functions? It might be the inline functions for c++ call these extern "C" functions for z/OS instead of having the z/OS function in the llvm namespace that returns std::error_code.

Copy link
Contributor Author

@abhina-sree abhina-sree Jun 10, 2025

Choose a reason for hiding this comment

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

I left those guarded for now, I checked the usages and the headers were not removed from those files since being added so it's not as urgent to refactor those. The C++ functions are calling the C functions under the covers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you simplify the functions some so that the general inline functions for C++ that return std::error_code call the z/OS functions for C that return an int. I was thinking:

std::error_code llvm::enableAutoConversion(int FD) {
#ifdef __MVS__
  if (::enablezOSAutoConversion(FD) == -1)
    return errnoAsErrorCode();
#endif

  return std::error_code();
}

This would eliminate the extra z/OS functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, this is done in the latest commit

int disablezOSAutoConversion(int FD);
int restorezOSStdHandleAutoConversion(int FD);

#ifdef __cplusplus
}
#endif /* __cplusplus */

#ifdef __cplusplus
namespace llvm {

inline std::error_code disableAutoConversion(int FD) {
#ifdef __MVS__
if (::disablezOSAutoConversion(FD) == -1)
return errnoAsErrorCode();
#endif
return std::error_code();
}

inline std::error_code enableAutoConversion(int FD) {
#ifdef __MVS__
if (::enablezOSAutoConversion(FD) == -1)
return errnoAsErrorCode();
#endif
return std::error_code();
}

inline std::error_code restoreStdHandleAutoConversion(int FD) {
#ifdef __MVS__
if (::restorezOSStdHandleAutoConversion(FD) == -1)
return errnoAsErrorCode();
#endif
return std::error_code();
}

inline std::error_code setFileTag(int FD, int CCSID, bool Text) {
#ifdef __MVS__
return setzOSFileTag(FD, CCSID, Text);
#endif
return std::error_code();
}

inline ErrorOr<bool> needConversion(const char *FileName, const int FD = -1) {
#ifdef __MVS__
return needzOSConversion(FileName, FD);
#endif
return false;
}

#ifdef __MVS__

/** \brief Disable the z/OS enhanced ASCII auto-conversion for the file
* descriptor.
*/
Expand All @@ -63,9 +106,8 @@ ErrorOr<__ccsid_t> getzOSFileTag(const char *FileName, const int FD = -1);
*/
ErrorOr<bool> needzOSConversion(const char *FileName, const int FD = -1);

#endif /* __MVS__*/
} /* namespace llvm */
#endif /* __cplusplus */

#endif /* __MVS__ */

#endif /* LLVM_SUPPORT_AUTOCONVERT_H */
21 changes: 0 additions & 21 deletions llvm/lib/Support/AutoConvert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,27 +83,6 @@ int enablezOSAutoConversion(int FD) {
return fcntl(FD, F_CONTROL_CVT, &Query);
}

std::error_code llvm::disablezOSAutoConversion(int FD) {
if (::disablezOSAutoConversion(FD) == -1)
return errnoAsErrorCode();

return std::error_code();
}

std::error_code llvm::enablezOSAutoConversion(int FD) {
if (::enablezOSAutoConversion(FD) == -1)
return errnoAsErrorCode();

return std::error_code();
}

std::error_code llvm::restorezOSStdHandleAutoConversion(int FD) {
if (::restorezOSStdHandleAutoConversion(FD) == -1)
return errnoAsErrorCode();

return std::error_code();
}

std::error_code llvm::setzOSFileTag(int FD, int CCSID, bool Text) {
assert((!Text || (CCSID != FT_UNTAGGED && CCSID != FT_BINARY)) &&
"FT_UNTAGGED and FT_BINARY are not allowed for text files");
Expand Down
30 changes: 19 additions & 11 deletions llvm/lib/Support/InitLLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,28 @@
#include "llvm/Support/Windows/WindowsSupport.h"
#endif

#ifdef __MVS__
#if defined(HAVE_UNISTD_H)
#include <unistd.h>
#else
#ifndef STDIN_FILENO
#define STDIN_FILENO 0
#endif
#ifndef STDOUT_FILENO
#define STDOUT_FILENO 1
#endif
#ifndef STDERR_FILENO
#define STDERR_FILENO 2
#endif
#endif

void CleanupStdHandles(void *Cookie) {
llvm::raw_ostream *Outs = &llvm::outs(), *Errs = &llvm::errs();
Outs->flush();
Errs->flush();
llvm::restorezOSStdHandleAutoConversion(STDIN_FILENO);
llvm::restorezOSStdHandleAutoConversion(STDOUT_FILENO);
llvm::restorezOSStdHandleAutoConversion(STDERR_FILENO);
llvm::restoreStdHandleAutoConversion(STDIN_FILENO);
llvm::restoreStdHandleAutoConversion(STDOUT_FILENO);
llvm::restoreStdHandleAutoConversion(STDERR_FILENO);
}
#endif

using namespace llvm;
using namespace llvm::sys;
Expand All @@ -41,10 +51,10 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,
assert(!Initialized && "InitLLVM was already initialized!");
Initialized = true;
#endif
#ifdef __MVS__

// Bring stdin/stdout/stderr into a known state.
sys::AddSignalHandler(CleanupStdHandles, nullptr);
#endif

if (InstallPipeSignalExitHandler)
// The pipe signal handler must be installed before any other handlers are
// registered. This is because the Unix \ref RegisterHandlers function does
Expand All @@ -68,8 +78,8 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,

// If turning on conversion for stderr fails then the error message
// may be garbled. There is no solution to this problem.
ExitOnErr(errorCodeToError(llvm::enablezOSAutoConversion(STDERR_FILENO)));
ExitOnErr(errorCodeToError(llvm::enablezOSAutoConversion(STDOUT_FILENO)));
ExitOnErr(errorCodeToError(llvm::enableAutoConversion(STDERR_FILENO)));
ExitOnErr(errorCodeToError(llvm::enableAutoConversion(STDOUT_FILENO)));
#endif

#ifdef _WIN32
Expand Down Expand Up @@ -97,8 +107,6 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,
}

InitLLVM::~InitLLVM() {
#ifdef __MVS__
CleanupStdHandles(nullptr);
#endif
llvm_shutdown();
}
10 changes: 4 additions & 6 deletions llvm/lib/Support/MemoryBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/Config/config.h"
#include "llvm/Support/Alignment.h"
#include "llvm/Support/AutoConvert.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorHandling.h"
Expand All @@ -34,9 +35,6 @@
#include <io.h>
#endif

#ifdef __MVS__
#include "llvm/Support/AutoConvert.h"
#endif
using namespace llvm;

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -508,15 +506,15 @@ getOpenFileImpl(sys::fs::file_t FD, const Twine &Filename, uint64_t FileSize,
}

#ifdef __MVS__
ErrorOr<bool> NeedConversion = needzOSConversion(Filename.str().c_str(), FD);
if (std::error_code EC = NeedConversion.getError())
ErrorOr<bool> NeedsConversion = needConversion(Filename.str().c_str(), FD);
if (std::error_code EC = NeedsConversion.getError())
return EC;
// File size may increase due to EBCDIC -> UTF-8 conversion, therefore we
// cannot trust the file size and we create the memory buffer by copying
// off the stream.
// Note: This only works with the assumption of reading a full file (i.e,
// Offset == 0 and MapSize == FileSize). Reading a file slice does not work.
if (Offset == 0 && MapSize == FileSize && *NeedConversion)
if (*NeedsConversion && Offset == 0 && MapSize == FileSize)
return getMemoryBufferForStream(FD, Filename);
#endif

Expand Down
19 changes: 11 additions & 8 deletions llvm/lib/Support/raw_ostream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -894,21 +894,24 @@ void raw_fd_ostream::anchor() {}
raw_fd_ostream &llvm::outs() {
// Set buffer settings to model stdout behavior.
std::error_code EC;
#ifdef __MVS__
EC = enablezOSAutoConversion(STDOUT_FILENO);
assert(!EC);
#endif

// On z/OS we need to enable auto conversion
static std::error_code EC1 = enableAutoConversion(STDOUT_FILENO);
assert(!EC1);
(void)EC1;

static raw_fd_ostream S("-", EC, sys::fs::OF_None);
assert(!EC);
return S;
}

raw_fd_ostream &llvm::errs() {
// Set standard error to be unbuffered.
#ifdef __MVS__
std::error_code EC = enablezOSAutoConversion(STDERR_FILENO);
// On z/OS we need to enable auto conversion
static std::error_code EC = enableAutoConversion(STDERR_FILENO);
assert(!EC);
#endif
(void)EC;

// Set standard error to be unbuffered.
static raw_fd_ostream S(STDERR_FILENO, false, true);
return S;
}
Expand Down