fhahn abandoned this revision.
fhahn added a comment.
My understanding is that https://reviews.llvm.org/D35436 prevents the case
described in this review from happening. Thanks for having a look!
https://reviews.llvm.org/D35081
___
cfe-commits
tejohnson added a comment.
In https://reviews.llvm.org/D35081#808789, @tejohnson wrote:
> In https://reviews.llvm.org/D35081#808517, @mehdi_amini wrote:
>
> > In https://reviews.llvm.org/D35081#808207, @tejohnson wrote:
> >
> > > My first option was if any copy is under the threshold, simply
tejohnson added a comment.
In https://reviews.llvm.org/D35081#808517, @mehdi_amini wrote:
> In https://reviews.llvm.org/D35081#808207, @tejohnson wrote:
>
> > My first option was if any copy is under the threshold, simply pick the
> > prevailing copy (it may be over threshold, but assume we
mehdi_amini added a comment.
In https://reviews.llvm.org/D35081#808207, @tejohnson wrote:
> My first option was if any copy is under the threshold, simply pick the
> prevailing copy (it may be over threshold, but assume we want that one
> anyway). Another possibility is to only allow importing
tejohnson added a comment.
In https://reviews.llvm.org/D35081#808204, @mehdi_amini wrote:
> In https://reviews.llvm.org/D35081#808189, @tejohnson wrote:
>
> > > Alternatively (and likely preferably from a compile-time point of view to
> > > limit the list of import), we could keep a map of
mehdi_amini added a comment.
In https://reviews.llvm.org/D35081#808189, @tejohnson wrote:
> > Alternatively (and likely preferably from a compile-time point of view to
> > limit the list of import), we could keep a map of GUID->Summary and reuse
> > it before trying to select a new callee.
>
>
tejohnson added a comment.
In https://reviews.llvm.org/D35081#808158, @mehdi_amini wrote:
> Thanks, very clear :)
>
> I would expect that if we reprocess a GUID we invalidate the previous import
> of the same GUID. Right now my impression is that the issue is that `
>
mehdi_amini added a comment.
Thanks, very clear :)
I would expect that if we reprocess a GUID we invalidate the previous import of
the same GUID. Right now my impression is that the issue is that `
ImportList[ExportModulePath][VI.getGUID()]` is indexed on the importing module.
So it'd require
tejohnson added a comment.
In https://reviews.llvm.org/D35081#808124, @mehdi_amini wrote:
> Teresa: can you describe a bit more how we end up importing two summaries for
> the same GUID? I would expect that after importing one, we don't even come to
> try to import another since we already
mehdi_amini added a comment.
Teresa: can you describe a bit more how we end up importing two summaries for
the same GUID? I would expect that after importing one, we don't even come to
try to import another since we already have one.
https://reviews.llvm.org/D35081
tejohnson added a comment.
In https://reviews.llvm.org/D35081#807497, @mehdi_amini wrote:
> In https://reviews.llvm.org/D35081#807172, @fhahn wrote:
>
> > It's @yunlian, so if the issue you described above covers his failure, than
> > that's great. @tejohnson do you have time to work on a fix
mehdi_amini added a comment.
In https://reviews.llvm.org/D35081#807172, @fhahn wrote:
> It's @yunlian, so if the issue you described above covers his failure, than
> that's great. @tejohnson do you have time to work on a fix soon? Otherwise I
> could give it a try, I would probably need a few
fhahn added a comment.
It's @yunlian, so if the issue you described above covers his failure, than
that's great. @tejohnson do you have time to work on a fix soon? Otherwise I
could give it a try, I would probably need a few pointers though, as I am not
really familiar with ThinLTO.
Passing
tejohnson added a comment.
In https://reviews.llvm.org/D35081#805761, @yunlian wrote:
> I've sent a reproduce test case to tejohnson.
FYI I tracked down what is going on here and reproduced with a small test case.
Essentially, two copies of a linkonce odr function were optimized somewhat
yunlian added a comment.
I've sent a reproduce test case to tejohnson.
https://reviews.llvm.org/D35081
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tejohnson added a comment.
In https://reviews.llvm.org/D35081#805506, @yunlian wrote:
> This error happens when I try to triage a thinLTO failure on ARM.
>
> Initially I got some error like
> Instruction does not dominate all uses!
>
> %205 = bitcast i1 (%"class.blink::LayoutObject"*)** %194
yunlian added a comment.
This error happens when I try to triage a thinLTO failure on ARM.
Initially I got some error like
Instruction does not dominate all uses!
%205 = bitcast i1 (%"class.blink::LayoutObject"*)** %194 to i8*, !dbg !51180
%200 = getelementptr i8, i8* %205, i32 ptrtoint
tejohnson added a comment.
In https://reviews.llvm.org/D35081#805127, @fhahn wrote:
> To reproduce the issue you could use
>
> +; Check that we only add a single summary entry for multiple definitions
> +; of a linkonce_odr function
> +
> +; RUN: opt -module-summary %s -o %t1.bc
> +;
fhahn added a comment.
To reproduce the issue you could use
+; Check that we only add a single summary entry for multiple definitions
+; of a linkonce_odr function
+
+; RUN: opt -module-summary %s -o %t1.bc
+; RUN: opt -module-summary %s -o %t2.bc
+; RUN: llvm-lto
tejohnson added a comment.
How are you invoking this? Typically this path is invoked for distributed
builds, when the individual index files for each backend were written during
the thin link (e.g. via gold plugin's -thinlto-index-only option). In that
case, we only expect a single summary to
fhahn created this revision.
Herald added a subscriber: inglorion.
When combining modules with entries that allow multiple definitions,
the combined index can contain multiple summaries for a single GUID.
Unless I miss something here, we should be able to continue by just
picking the first
21 matches
Mail list logo
