Rafael Espíndola
2014-Sep-19 20:01 UTC
[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
Steven Wu
2014-Sep-19 21:08 UTC
[LLVMdev] [RFC] Exhaustive bitcode compatibility tests for IR features
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
Sergei Larin
2014-Oct-02 16:05 UTC
[LLVMdev] [RFC] Exhaustive bitcode compatibility tests for IR features
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
Rafael Espíndola
2014-Oct-07 05:33 UTC
[LLVMdev] [RFC] Exhaustive bitcode compatibility tests for IR features
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