[PATCH] D78403: Infer alignment of loads with unspecified alignment in IR/bitcode parsing.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/include/llvm/AsmParser/Parser.h:31
 
+typedef llvm::function_ref<void (Module*)> DataLayoutCallbackTy;
+
----------------
mehdi_amini wrote:
> efriedma wrote:
> > mehdi_amini wrote:
> > > The type for the callback is strange to me, this allows basically any modification of the Module during the callback, can't we just pass an `Optional<DataLayout> override_datalayout` in the parsing functions?
> > llc needs access to the target triple from the IR file to decide which datalayout to use.  Hence the callback API: we need to decide the datalayout after we parse the triple, but before we parse any function definitions.
> In this case can you make it a callback that would be something like `DataLayout callback(Triple&)`?
Nit: function argument shouldn't take a string by copy, ideally should StringRef.


================
Comment at: llvm/include/llvm/AsmParser/Parser.h:95
 ///                         for use inside the LLVM testuite!
 /// \param DataLayoutString Override datalayout in the llvm assembly.
+ParsedModuleAndIndex parseAssemblyFileWithIndex(
----------------
Can you update the doxygen? (same for the other functions)



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78403/new/

https://reviews.llvm.org/D78403