[cfe-commits] r65746 - in /cfe/trunk: lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGExprConstant.cpp lib/CodeGen/CodeGenFunction.h test/CodeGen/blocks.c

[Anders Carlsson]

On Mar 1, 2009, at 11:12 AM, Mike Stump wrote:

> On Mar 1, 2009, at 10:58 AM, Anders Carlsson wrote:
>> 1 mar 2009 kl. 10.51 skrev Mike Stump:
>>
>>> On Feb 28, 2009, at 5:09 PM, Anders Carlsson wrote:
>>>> +  // Check if the block can be global.
>>>> +  if (CanBlockBeGlobal(Info))
>>>> +    return CGM.GetAddrOfGlobalBlock(BE, Name.c_str());
>>>
>>> Like the last change, this also breaks things that were  
>>> working.  :-(  Please, monotonic improvements only please.  We  
>>> lazily create imports during codegen and your code doesn't account  
>>> for that.
>>
>> I think it is better to just use one code path for creating global  
>> blocks.
>
> Why?  It replicates the other code entirely.

If one code path replicates the other code path, then why have two  
code paths in the first place? I think at this stage it's simpler to  
look for the block variables beforehand - it simplifies the  
"Unsupported" checks and it also makes code like

void f() {
	int a;

	^{
		^{
			a;
		};
	};
}

easier to deal with. In the future we could even be clever and sort  
imported block variables by size/alignment to minimize memory wasted  
in "holes".


>> I added code to look up imports beforehand.
>
> And my point it, is is wrong.  It is slow, I don't want to walk  
> around looking for all the data you walk around for and it is  
> unnecessary and you broke previously working code like:
>

Traversing isn't that slow...we could even make it only traverse  
blocks that we know have variables.

(If by "broke" you mean that it now gives an "unsupported" error, then  
yes I broke it ;)

Anders