[PATCH] D159212: [MLIR] Allow dialects to disable CSE for certain operations

jsjodin added a comment. I'm not sure if we want to add an interface for every transform. Is there a way to have a generic solution to this, like a new trait or more general interface? It would be nice to avoid the case where a new transform will have to touch all the dialects, or a new op

[PATCH] D151035: [OpenMP][OpenMPIRBuilder] Migrate kernel launch code and host fallback code generation from Clang to the OpenMPIRBuilder

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGac65cc121513: [OpenMP][OpenMPIRBuilder] Migrate kernel launch code and host fallback code… (authored by jsjodin). Herald added a project: clang.

[PATCH] D147217: [OpenMP][OMPIRBuilder] OpenMPIRBuilder support for requires directive

jsjodin accepted this revision. jsjodin added a comment. This revision is now accepted and ready to land. I think this is fine. The dependent diff has already been approved. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147217/new/

[PATCH] D150860: [OpenMP] Change clang emitTargetDataCalls to use OMPIRBuilder

jsjodin added inline comments. Comment at: clang/test/OpenMP/target_data_codegen.cpp:355-356 // Region 00 +// CK2-DAG: [[DEV:%[^,]+]] = sext i32 [[DEVi32:%[^,]+]] to i64 +// CK2-DAG: [[DEVi32]] = load i32, ptr %{{[^,]+}}, // CK2: br i1 %{{[^,]+}}, label %[[IFTHEN:[^,]+]],

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

jsjodin accepted this revision. jsjodin added a comment. This revision is now accepted and ready to land. Looks good. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:5230 + if (!IsExternallyVisible) +OS << llvm::format("_%x", EntryInfo.FileID); + OS <<

[PATCH] D147217: [OpenMP][OMPIRBuilder] OpenMPIRBuilder support for requires directive

jsjodin added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:5291 +Function *OpenMPIRBuilder::createRegisterRequires(StringRef Name) { + // Skip the creation of the registration function if this is device codegen Perhaps this function

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

jsjodin added a comment. @jdoerfert do you have any feedback? Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1882 + auto EntryInfo = OMPBuilder.getTargetEntryUniqueInfo( + PLoc.getFilename(), PLoc.getLine(), VD->getName()); SmallString<128> Buffer, Out;

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

jsjodin added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:865 + TargetRegionEntryInfo EntryInfo, StringRef MangledName, + Module *LlvmModule, std::vector , + bool OpenMPSIMD, std::vector TargetTriple, Instead of

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

jsjodin added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4376 +function_ref CustomMapperCB) { + auto EmitNonContiguousDescriptor = [&]() { +MapInfosTy::StructNonContiguousInfo = I don't think we need a lambda function, it

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

jsjodin requested changes to this revision. jsjodin added inline comments. This revision now requires changes to proceed. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:817 + /// LLVMModule. + llvm::Constant *getAddrOfDeclareTargetVar( +

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

jsjodin added inline comments. Comment at: mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h:174 +if (offloadMod.getIsDevice()) { + hostIRFilePath = offloadMod.getHostIRFilePath(); +} agozillon wrote: > This will break the patch

[PATCH] D146549: [OpenMP][OMPIRBuilder] Make OffloadEntriesInfoManager a member of OpenMPIRBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rG85faee69928b: [OpenMP][OMPIRBuilder] Make OffloadEntriesInfoManager a member of… (authored by jsjodin). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

jsjodin added a comment. In D142244#4096777 , @clementval wrote: > In D142244#4096648 , @jsjodin wrote: > >> In D142244#4096579 , @clementval >> wrote: >> >>> In

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

jsjodin added a comment. In D142244#4096579 , @clementval wrote: > In D142244#4096546 , @jsjodin wrote: > >> In D142244#4096538 , @clementval >> wrote: >> >>> In

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

jsjodin added a comment. In D142244#4096538 , @clementval wrote: > In D142244#4096512 , @jsjodin wrote: > >> No, it should work for any configuration as far as I know. How are you >> running the test? > > Just

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

jsjodin added a comment. In D142244#4096492 , @clementval wrote: > The new test embed.f90 and embed-error.f90 are failing on my side. Do they > assume a specific configuration? No, it should work for any configuration as far as I know. How are you

[PATCH] D142244: [flang][driver] Add support for -embed-offload-object flag in flang -fc1

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG40d8c0666f19: [flang][driver] Add support for -embed-offload-object flag in flang (authored by jsjodin). Herald added projects: clang, Flang. Herald

[PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

jsjodin accepted this revision. jsjodin added a comment. This revision is now accepted and ready to land. This looks good to me. Have @jdoerfert approve as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141910/new/

[PATCH] D139634: [OpenMP][OMPIRBuilder] Migrate code to emit target region functions from clang to OMPIRBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rG1f8fecf26b35: [OpenMP][OMPIRBuilder] Migrate code to emit target region functions from clang… (authored by jsjodin). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior

[PATCH] D137587: [OpenMP][OMPIRBuilder] Migrate target outlined function registration to OMPIRBuilder from clang

This revision was automatically updated to reflect the committed changes. Closed by commit rG2166d9529a60: [OpenMP][OMPIRBuilder] Migrate target outlined function registration to… (authored by jsjodin). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM

[PATCH] D137725: [OpenMP][OMPIRBuilder] Mirgrate getName from clang to OMPIRBuilder

This revision was automatically updated to reflect the committed changes. Closed by commit rG2aa338f68e1e: [OpenMP][OMPIRBuilder] Mirgrate getName from clang to OMPIRBuilder (authored by jsjodin). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github

[PATCH] D138220: [OpenMP][OMPIRBuilder] Add a configuration class to captures flags/attributes that affect codegen

This revision was automatically updated to reflect the committed changes. Closed by commit rG969d787a470a: [OpenMP][OMPIRBuilder] Add a configuration class that captures flags that… (authored by jsjodin). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG

[PATCH] D136872: [OpenMP][OpenMPIRBuilder] Migrate loadOffloadInfoMetadata from clang to OMPIRbuilder

jsjodin added a comment. @jdoerfert do you have any other comments? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136872/new/ https://reviews.llvm.org/D136872 ___ cfe-commits mailing list

[PATCH] D136872: [OpenMP][OpenMPIRBuilder] Migrate loadOffloadInfoMetadata from clang to OMPIRbuilder

jsjodin added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4712 +Module , OffloadEntriesInfoManager , +bool isDevice) { + // If we are in target mode, load the metadata from the host IR. This code has isDevice must be true

[PATCH] D136601: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region info

This revision was automatically updated to reflect the committed changes. Closed by commit rG67f8521cd4b6: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region… (authored by jsjodin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136601: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region info

jsjodin updated this revision to Diff 471611. jsjodin added a comment. Remove debug code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136601/new/ https://reviews.llvm.org/D136601 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/test/OpenMP/declare_target_codegen.cpp

[PATCH] D136601: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region info

jsjodin added a comment. What is the procedure to redo a patch that was reverted? Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1909 +CtorEntryInfo.ParentName = Twine(Buffer, "_ctor").toStringRef(Out); +llvm::errs() << "Registering var ctor: " << Twine(Buffer,

[PATCH] D136601: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region info

jsjodin updated this revision to Diff 471610. jsjodin added a comment. Fix reference error. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136601/new/ https://reviews.llvm.org/D136601 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/test/OpenMP/declare_target_codegen.cpp

[PATCH] D136601: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region info

jsjodin added a comment. In D136601#3889822 , @kda wrote: > The bracket builds have finished and they seem to have a conclusive result. > The test starts failing with this patch. > > Built at this patch:

[PATCH] D136601: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region info

jsjodin added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3031 +OrderedEntries[E.getOrder()] = +std::make_tuple(, Loc, EntryInfo.ParentName); +ParentFunctions[E.getOrder()] = EntryInfo.ParentName; mikerice

[PATCH] D136601: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region info

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3d0e9edd8e53: [OpenMP] [OMPIRBuilder] Create a new datatype to hold the unique target region… (authored by jsjodin). Herald added a project: clang.

[PATCH] D134662: [OpenMPIRBuilder] Migrate emitOffloadingArraysArgument from clang

This revision was automatically updated to reflect the committed changes. Closed by commit rG4627cef1134f: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArraysArgument from clang (authored by jsjodin). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM