[SYCL][NFC] Remove makeResultFilename function from sycl-post-link #17602
Conversation
The functionality of the makeResultFilename function has been moved to the getOutputPrefix function, and it is invoked at the beginning of the program. Two SaveModuleIR functions have been merged into one function. The output setup in the saveModuleProperties function has been moved out of the function for simplification. Two dead ifs have been removed. This simplifying change is mandatory for the later migration of IO utilities of sycl-post-link to the library.
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
Update: My original statement in the comment "OutputFiless size is never zero" turned out to be false and subsequent refactorings weren't correct. Therefore, I removed changes related to @maarquitos14 Could you please rereview this PR? |
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
| } | ||
|
|
||
| void saveModuleIR(Module &M, StringRef OutFilename) { | ||
| void saveModuleIR(Module &M, const StringRef Filename) { |
There was a problem hiding this comment.
| void saveModuleIR(Module &M, const StringRef Filename) { | |
| void saveModuleIR(const Module &M, const StringRef Filename) { |
There was a problem hiding this comment.
MPM.run would not accept a const reference.
| // the result. | ||
| void saveModule(std::vector<std::unique_ptr<util::SimpleTable>> &OutTables, | ||
| module_split::ModuleDesc &MD, int I, StringRef IRFilename) { | ||
| /// \param OutTables List of tables (one for each target) to output results |
| void saveModuleSymbolTable(const module_split::ModuleDesc &MD, | ||
| const StringRef Filename) { | ||
| std::string SymT = computeModuleSymbolTable(MD.getModule(), MD.entries()); | ||
| writeToFile(Filename, SymT); | ||
| } | ||
|
|
||
| // Compute the filename suffix for the module |
There was a problem hiding this comment.
Can this be merged with getOutputSuffix?
There was a problem hiding this comment.
We have only getOutputPrefix at the moment. I've just noticed that we don't need the function getModuleSuffix being used only once.
| std::string NewSuff = Suffix.str(); | ||
| if (!Target.empty()) | ||
| NewSuff = (Twine("_") + Target).str(); | ||
|
|
||
| CopyTriple.Prop = |
There was a problem hiding this comment.
Curious. Why do we not use DoPropGen flag here to guard 'saveModuleProperties'?
There was a problem hiding this comment.
Looks like we do use the flag.
| } | ||
|
|
||
| ++ID; | ||
| } | ||
| } | ||
|
|
||
| if (IsBF16DeviceLibUsed && (DeviceLibDir.getNumOccurrences() > 0)) { | ||
| saveDeviceLibModule(Tables, ID, "libsycl-fallback-bfloat16.bc"); | ||
| saveDeviceLibModule(Tables, ID + 1, "libsycl-native-bfloat16.bc"); | ||
| saveDeviceLibModule(Tables, OutputPrefix, ID, |
There was a problem hiding this comment.
It might be cleaner to populate a global constant with the list of device library module filenames and use that here? Ideally, it should be passed in as an option to sycl-post-link.
There was a problem hiding this comment.
We could leave that for later.
maksimsab
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
|
Hi @intel/llvm-gatekeepers ! |
The functionality of the makeResultFilename function has been moved to the getOutputPrefix function, and it is invoked at the beginning of the program.
Two SaveModuleIR functions have been merged into one function.
The output setup in the saveModuleProperties function has been moved out of the function for simplification.
This simplifying change is mandatory for the later migration of IO utilities of sycl-post-link to the library.