To be clear, the changes we discussed have been committed, but
now there is some special handling for FLAG_EXTERN + FLAG_TYPE_SYMBOL
in the LLVM backend in order to handle this business with sizeof.
Saw that hours after responding -- sorry for the noise.
Yes, sizeof(c_int) passes a VarSymbol with FLAG_TYPE_SYMBOL as you guessed.
(Still got it!)
I continue to have the position that the code generator should not need
to know about type symbols; something is either a Type or a Variable...
That's a valid opinion, but it's still not clear to me whether you're
proposing:
* deprecating the ability to have extern functions that take type
arguments (like sizeof())
or
* representing calls like 'sizeof(c_int)' in a different way within the
IR (in which case, how are you proposing representing them?)
Branch predicting that you might be proposing the former:
I think that supporting extern functions with type arguments (as we do
currently) can be a powerful feature, particularly if you want to target a
C macro that takes types as arguments; I realize in saying this that it's
a fairly C-back-end-centric point-of-view.
If we did deprecate the general support for this capability, would you
propose preserving sizeof() in some special way for developers to use as
we currently do or...?
Thanks,
-Brad
-michael
On 08/25/2014 01:42 PM, Brad Chamberlain wrote:
I was hoping someone else would wrap up this thread before I got back from
vacation. No such luck... :)
I'm still not going to be able to answer your overarching question without
more effort, but chiming in on the can of worms that I opened:
Generally speaking, I think you're right that type symbols shouldn't have
much presence in the generated code, at least for standard types like ints,
because they should be resolved statically. Offhand, I don't know the
answer to whether types with a runtime component (like arrays) have a
greater need to preserve the type through to codegen time -- I'd need to
think about that more.
I do know that there is a capability to have an extern procedure take a
type argument, such that sizeof() can be prototyped, for example. And for
this to work, presumably the type's symbol would need to be code-generated.
I used this trick in your util/config/makeSysBasicTypes file, for example,
to ensure that we got the sizes correct for all of the c_* type aliases.
If I weren't catching up on two weeks of email, I'd take a look at the IR
to see if the expressions passed to the sizeof() calls in the resulting
gen/*/SysCTypes.chpl module (like 'c_int' in 'sizeof(c_int)') are
VarSymbols with FLAG_TYPE_SYMBOL attached. From your mail, I'd guess they
are. Whether or not they should be is a reasonable question.
-Brad
On Tue, 12 Aug 2014, [email protected] wrote:
Brad -
Yes, right now
extern type c_int = int(32);
results in c_int being a VarSymbol with FLAG_TYPE_VARIABLE.
That doesn't really make sense to me though since by the time we get to
code generation, what should the code generator do with FLAG_TYPE_VARIABLE
VarSymbols? Are these always types? (or sometimes "runtime types" which
are actually values)?
If they're always types, there's not really any point in doing the usual
variable things... e.g. allocating on the stack or assigning to them ...
but I saw the code generator generating code that assigns to a VarSymbol
with FLAG_TYPE_VARIABLE, and turning it off makes arrays of arrays no
longer work... so the whole situation doesn't make any sense to me.
But, it would help me (and maybe others) if somebody would clarify what
VarSymbol + FLAG_TYPE_VARIABLE is used for (vs just being a type) and what
the code generator should do with such things. Is it sometimes a runtime
type and sometimes not? If so, is there a way we can or should distinguish
between those two cases?
Thanks,
-michael
________________________________________
From: Brad Chamberlain [[email protected]]
Sent: Friday, August 08, 2014 18:32
To: [email protected]
Cc: [email protected]
Subject: Re: [Chapel-developers] module init functions
I haven't been following this closely, but Michael's FLAG* question jumped
out at me. Wouldn't:
Chapel:
extern type age = c_int;
where C's header said:
typedef int age;
result in Chapel's notion of 'age' having both FLAG_TYPE_VARIABLE and
FLAG_EXTERN on it?
-Brad
On Fri, 8 Aug 2014, Michael Ferguson wrote:
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
------------------------------------------------------------------------------
Slashdot TV.
Video for Nerds. Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers
