Hi - See my pull request - I've backed out the changes for FLAG_TYPE_VARIABLE VarSymbols, so now we just need Sung (or somebody) to review the --about changes.
I still don't understand why FLAG_TYPE_VARIABLE + FLAG_EXTERN makes any sense though.... -michael On 08/08/2014 12:36 PM, Michael Ferguson wrote: > Hi Mike - > > They are already separate requests... obviously since they are separate > commits. > Somebody reviewing one commit should just review that commit. > > Without *all* of these fixes, the LLVM backend does not work. That's why I > consider > it one pull request. The patches can still be reviewed separately. > > For the changes in "Handle type variables in a few special places" and > "Don't heap register type variables" > - I don't understand what changes caused this to happen, but now we > are getting type variables doing things *in codegen*. Since they are > types, I would expect them to be gone by codegen... The code generator > tries to generate someTypeVariable = somethingElse. I have no idea why > it works in the C backend and I think this kind of thing is in error. > I think the LLVM backend just complains more loudly about weird things. > - Some of these changes are specific to LLVM (to make sizeof(c_int) work). > It used to work (unmodified) and again I havn't done archeology to > find out what broke, but it seems that the broke thing needs fixing > in any case. > - Somebody who understands type variables should review those changes > > -michael > > On 08/07/2014 07:52 PM, Mike Noakes wrote: >> >> On Aug 7, 2014, at 2:44 PM, Michael Ferguson <[email protected]> wrote: >> >>> Hi - >>> >>> OK, I have pull request #124 about this, >>> but: >>> >>> I was very surprised to see >>> symbols with FLAG_TYPE_VARIABLE ending up all the way >>> in codegen. In particular, I'd recommend that somebody >>> add an assertion failure near "Why are we getting here?" >>> around expr.cpp:3721 in the patched version -- I don't >>> think that code should be necessary. >>> >>> >>> Thanks, >>> >>> -michael >> >> >> Michael, >> >> [This is direct email rather than GIT-HUB comments]. >> >> >> >> I took a look at this pull request but I am not certain I understand the >> scope >> for some of the changes. It seems to me that this request touches more than >> one issue but I am not certain how many issues are involved. >> >> Tom and I took a look at this and we concluded that I should seek some >> clarification. >> >> >> 1) The “fix up —about” that you suggest Sung look at seems to be a separable >> issue. Is that right? >> >> It might have been slightly easier if this had been in its own pull-request >> so that it >> could be reviewed/applied more easily. >> >> Depending on the scope of the other changes we may be able to handle this on >> our side or we might ask for a separate pull-request. >> >> >> 2) There’s a trivial change in getIntermediateDirName() that is easy to >> understand >> and approve. >> >> >> 3) I understand the change you made in externCResolve. There are two >> relatively simple changes for code that is conditional on LLVM. It is easy >> to approve >> and merge these changes. >> >> >> 4) So finally there are the other two commits. We are unsure if this is an >> effort >> to fix a different issue that you became aware of or if this is a proposed >> solution >> for some other consequences of moving the initFun stuff that we haven’t >> appreciated. >> >> A yellow flag for us is that the change in expr.cpp appears to run for both >> LLVM and >> STD but we don’t know of a problem in STD that this change is trying to >> address. >> >> Could you provide some additional background? >> >> With thanks and regards, >> >> Mike >> >> >> >> >> >>> >>> On 08/07/2014 01:11 PM, Michael Ferguson wrote: >>>> Hi Mike - >>>> >>>> >>>> Thanks very much for your help! >>>> >>>> module->block->insertAtHead(result) seems to get the job done. I'm doing >>>> more testing now, but expect a pull request from me soon fixing this >>>> and maybe another LLVM problem. >>>> >>>> (We really need to get nightly/weekly LLVM testing going somehow...) >>>> >>>> Best, >>>> >>>> -michael >>>> >>>> On 08/07/2014 12:26 PM, Mike Noakes wrote: >>>>> >>>>> On Aug 7, 2014, at 8:41 AM, Michael Ferguson <[email protected]> wrote: >>>>> >>>>>> Hi - >>>>>> >>>>>> I've noticed that >>>>>> chpl --print-passes test/extern/ferguson/externblock/define.chpl >>>>>> now core dumps in the compiler because >>>>>> the extern block support tries to do >>>>>> module->initFn->insertAtHead(result); >>>>>> when trying to add a global variable, >>>>>> but module->initFn is NULL. >>>>>> >>>>>> This used to work but I recall seeing some recent >>>>>> changes to module initialization. >>>>>> >>>>>> Since readExternC runs fairly early on >>>>>> (so that it can happen before scopeResolve), >>>>>> it sometimes changes the AST in odd ways. >>>>>> How should it add a global variable? >>>>>> (Is there a way to request that the module's >>>>>> init function be created, for example? Or >>>>>> should it be doing something else?) >>>>>> >>>>>> Thanks, >>>>>> >>>>>> -michael >>>>> >>>>> >>>>> Hi Michael, >>>>> >>>>> I did the work to relocate the code that inserts Module Init functions >>>>> from Parse >>>>> to Normalize. >>>>> >>>>> I was surprised to see that there is a test that fails but now I see that >>>>> it relies on LLVM. >>>>> Apparently we haven't done a run with LLVM since I completed that work. >>>>> >>>>> 1) I'd be happy to take on the work to apply the required changes to >>>>> enable the LLVM >>>>> build to work with this change. >>>>> >>>>> 2) Alternatively you might find it is not terribly hard. There is a fair >>>>> chance that you >>>>> will be able to replace "module->initFn->insertAtHead(result)" with >>>>> "module->block->insertAtHead(result)" >>>>> >>>>> >>>>> >>>>> By way of a little background: >>>>> >>>>> Historically the parser collected the statements in the source level code >>>>> in to the block >>>>> and then ran a final pass in which the AST for a "module init function" >>>>> is created and >>>>> the most of the original contents of the block were inserted in to the >>>>> body of that function >>>>> (there are a few exceptions). >>>>> >>>>> During Normalize, most of body of the init function was pulled back out >>>>> to the Module >>>>> level leaving only the code necessary to initialize the module-level >>>>> variables etc. >>>>> >>>>> The passes that ran between Parse and Normalize had to "be aware" that >>>>> most of the >>>>> module was buried inside inside the Module init function; as the function >>>>> you are considering >>>>> appears to do. >>>>> >>>>> >>>>> >>>>> If you are willing I'd like to propose that you take the first step at >>>>> the proposed change but >>>>> I will be very happy to take over if it is not a relatively simple change. >>>>> >>>>> With regards, >>>>> >>>>> Mike >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>>>> >>>>>> ------------------------------------------------------------------------------ >>>>>> Infragistics Professional >>>>>> Build stunning WinForms apps today! >>>>>> Reboot your WinForms applications with our WinForms controls. >>>>>> Build a bridge from your legacy apps to the future. >>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk >>>>>> _______________________________________________ >>>>>> Chapel-developers mailing list >>>>>> [email protected] >>>>>> https://lists.sourceforge.net/lists/listinfo/chapel-developers >>>>> >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> Infragistics Professional >>>> Build stunning WinForms apps today! >>>> Reboot your WinForms applications with our WinForms controls. >>>> Build a bridge from your legacy apps to the future. >>>> http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk >>>> _______________________________________________ >>>> Chapel-developers mailing list >>>> [email protected] >>>> https://lists.sourceforge.net/lists/listinfo/chapel-developers >>>> >>> >>> >>> ------------------------------------------------------------------------------ >>> Infragistics Professional >>> Build stunning WinForms apps today! >>> Reboot your WinForms applications with our WinForms controls. >>> Build a bridge from your legacy apps to the future. >>> http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk >>> _______________________________________________ >>> Chapel-developers mailing list >>> [email protected] >>> https://lists.sourceforge.net/lists/listinfo/chapel-developers >> > > > ------------------------------------------------------------------------------ > Want fast and easy access to all the code in your enterprise? Index and > search up to 200,000 lines of code with a free copy of Black Duck > Code Sight - the same software that powers the world's largest code > search on Ohloh, the Black Duck Open Hub! Try it now. > http://p.sf.net/sfu/bds > _______________________________________________ > Chapel-developers mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/chapel-developers > ------------------------------------------------------------------------------ Want fast and easy access to all the code in your enterprise? Index and search up to 200,000 lines of code with a free copy of Black Duck Code Sight - the same software that powers the world's largest code search on Ohloh, the Black Duck Open Hub! Try it now. http://p.sf.net/sfu/bds _______________________________________________ Chapel-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/chapel-developers
