[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

2020-09-28 Thread Axel Y. Rivera via Phabricator via cfe-commits
ayrivera added inline comments.



Comment at: lld/ELF/Driver.cpp:895
   const char *argv[] = {config->progName.data(), opt.data()};
+  cl::ResetAllOptionOccurrences();
   if (cl::ParseCommandLineOptions(2, argv, "", &os))

Hi,

I built locally lld and came across an issue. The ELF driver isn't recognizing 
multiple -mllvm options. It only process the last -mllvm option that was 
entered in the command line. For example, I can add -mllvm -print-after-all 
-debug-pass=Arguments and it only prints the information from 
-debug-pass=Arguments, and not the IR and the debug data. If I swap the order, 
then the IR is printed and not the debug information.

I noticed that this change set added a call to cl:ResetAllOptionOccurrences() 
(line 895) inside parseClangOption. The function parseClangOption is called 
inside the loop that process the -mllvm options. If I comment this line, then 
everything works correctly and I can pass multiple -mllvm options through the 
command.

Is this an intended behavior or an actual issue? Thanks for your time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

2020-09-28 Thread Axel Y. Rivera via Phabricator via cfe-commits
ayrivera added inline comments.



Comment at: lld/ELF/Driver.cpp:895
   const char *argv[] = {config->progName.data(), opt.data()};
+  cl::ResetAllOptionOccurrences();
   if (cl::ParseCommandLineOptions(2, argv, "", &os))

MaskRay wrote:
> ayrivera wrote:
> > Hi,
> > 
> > I built locally lld and came across an issue. The ELF driver isn't 
> > recognizing multiple -mllvm options. It only process the last -mllvm option 
> > that was entered in the command line. For example, I can add -mllvm 
> > -print-after-all -debug-pass=Arguments and it only prints the information 
> > from -debug-pass=Arguments, and not the IR and the debug data. If I swap 
> > the order, then the IR is printed and not the debug information.
> > 
> > I noticed that this change set added a call to 
> > cl:ResetAllOptionOccurrences() (line 895) inside parseClangOption. The 
> > function parseClangOption is called inside the loop that process the -mllvm 
> > options. If I comment this line, then everything works correctly and I can 
> > pass multiple -mllvm options through the command.
> > 
> > Is this an intended behavior or an actual issue? Thanks for your time.
> Hi,
> 
> > For example, I can add -mllvm -print-after-all -debug-pass=Arguments
> 
> You need two -mllvm, i.e. `-mllvm -print-after-all -mllvm 
> -debug-pass=Arguments`. And with the option, I see the print-after-all dump.
Hi,


> You need two -mllvm, i.e. -mllvm -print-after-all -mllvm 
> -debug-pass=Arguments. And with the option, I see the print-after-all dump.


Thanks for the quick reply. Sorry, this is a typo. I'm using both -mllvm and 
still got the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

2020-09-29 Thread Axel Y. Rivera via Phabricator via cfe-commits
ayrivera added inline comments.



Comment at: lld/ELF/Driver.cpp:895
   const char *argv[] = {config->progName.data(), opt.data()};
+  cl::ResetAllOptionOccurrences();
   if (cl::ParseCommandLineOptions(2, argv, "", &os))

aganea wrote:
> ayrivera wrote:
> > MaskRay wrote:
> > > ayrivera wrote:
> > > > Hi,
> > > > 
> > > > I built locally lld and came across an issue. The ELF driver isn't 
> > > > recognizing multiple -mllvm options. It only process the last -mllvm 
> > > > option that was entered in the command line. For example, I can add 
> > > > -mllvm -print-after-all -debug-pass=Arguments and it only prints the 
> > > > information from -debug-pass=Arguments, and not the IR and the debug 
> > > > data. If I swap the order, then the IR is printed and not the debug 
> > > > information.
> > > > 
> > > > I noticed that this change set added a call to 
> > > > cl:ResetAllOptionOccurrences() (line 895) inside parseClangOption. The 
> > > > function parseClangOption is called inside the loop that process the 
> > > > -mllvm options. If I comment this line, then everything works correctly 
> > > > and I can pass multiple -mllvm options through the command.
> > > > 
> > > > Is this an intended behavior or an actual issue? Thanks for your time.
> > > Hi,
> > > 
> > > > For example, I can add -mllvm -print-after-all -debug-pass=Arguments
> > > 
> > > You need two -mllvm, i.e. `-mllvm -print-after-all -mllvm 
> > > -debug-pass=Arguments`. And with the option, I see the print-after-all 
> > > dump.
> > Hi,
> > 
> > 
> > > You need two -mllvm, i.e. -mllvm -print-after-all -mllvm 
> > > -debug-pass=Arguments. And with the option, I see the print-after-all 
> > > dump.
> > 
> > 
> > Thanks for the quick reply. Sorry, this is a typo. I'm using both -mllvm 
> > and still got the issue.
> > Is this an intended behavior or an actual issue?
> At first sight, this is an actual issue. Normally, 
> `cl::ResetAllOptionOccurrences()` should be called only once per compiler or 
> linker instance, to reset the internal counters for the `cl::opt`s. I would 
> move it somewhere at the begining of `readConfigs()`. Can you try that see if 
> that solves your issue?
Hi @aganea ,

This patch https://reviews.llvm.org/D88461 , posted by @MaskRay , works. It 
does what you suggested. Thanks for the quick reply.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits