llvm dev - Oct 2014 - [LLVMdev] [RFC] Exhaustive bitcode compatibility tests for IR features

On 19 September 2014 15:54, Steven Wu <stevenwu at apple.com>
wrote:
> Yes, we don’t need to edit the assembly in the file, but we need to
modified the CHECK line to reflect the output of current llvm-dis. I was talking
about updating the CHECK in all the previous version. Does that make sense?
Yes, the CHECK lines would have to be updated, but that seems like a
pretty small annoyance.

Changing the assembly format means updating all tests that produce it,
including test/Transforms and clang's tests, so this would be a drop
in the ocean.

Cheers,
Rafael

Make senses. Thank Rafael for the feedback. I will proceed to finish the test.
> On Sep 19, 2014, at 1:01 PM, Rafael Espíndola <rafael.espindola at
gmail.com> wrote:
> 
> On 19 September 2014 15:54, Steven Wu <stevenwu at apple.com> wrote:
>> Yes, we don’t need to edit the assembly in the file, but we need to
modified the CHECK line to reflect the output of current llvm-dis. I was talking
about updating the CHECK in all the previous version. Does that make sense?
> 
> Yes, the CHECK lines would have to be updated, but that seems like a
> pretty small annoyance.
> 
> Changing the assembly format means updating all tests that produce it,
> including test/Transforms and clang's tests, so this would be a drop
> in the ocean.
> 
> Cheers,
> Rafael

Hi Rafael,

  I have a quick question for you. First of all I am not very familiar with this
code, so....


Before this change: 

[llvm] r212349 - Implement LTOModule on top of IRObjectFile
http://comments.gmane.org/gmane.comp.compilers.llvm.cvs/195450


LTOModule::parseSymbols used to explicitly add global aliases as defined
symbols:

  // add aliases
 for (const auto &Alias : IRFile->aliases())
    addDefinedDataSymbol(Alias);

After your patch I do not seem to see an explicit handling of aliases... I would
naively expect something like this:

for (auto &Sym : IRFile->symbols()) {
.....
    if (isa<GlobalVariable>(GV)) {
      addDefinedDataSymbol(Sym);
      continue;
    }
    
    if (isa<GlobalAlias>(GV)) {
      addDefinedDataSymbol(Sym);
      continue;
    }

}


 Is this an oversight or a design decision? If this is the later, when/where
aliases should be handled?
Thanks a lot.

Sergei




---
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The
Linux Foundation

On 2 October 2014 12:05, Sergei Larin <slarin at codeaurora.org>
wrote:
>
> Hi Rafael,
>
>   I have a quick question for you. First of all I am not very familiar with
this code, so....
>
>
> Before this change:
>
> [llvm] r212349 - Implement LTOModule on top of IRObjectFile
> http://comments.gmane.org/gmane.comp.compilers.llvm.cvs/195450
>
>
> LTOModule::parseSymbols used to explicitly add global aliases as defined
symbols:
>
>   // add aliases
>  for (const auto &Alias : IRFile->aliases())
>     addDefinedDataSymbol(Alias);
>
> After your patch I do not seem to see an explicit handling of aliases... I
would naively expect something like this:
>
> for (auto &Sym : IRFile->symbols()) {
> .....
>     if (isa<GlobalVariable>(GV)) {
>       addDefinedDataSymbol(Sym);
>       continue;
>     }
>
>     if (isa<GlobalAlias>(GV)) {
>       addDefinedDataSymbol(Sym);
>       continue;
>     }
It looks almost like that:

557 if (F) {
558 addDefinedFunctionSymbol(Sym);
559 continue;
560 }
561
562 if (isa<GlobalVariable>(GV)) {
563 addDefinedDataSymbol(Sym);
564 continue;
565 }
566
567 assert(isa<GlobalAlias>(GV));
568 addDefinedDataSymbol(Sym);

Cheers,
Rafael