[llvm-commits] [llvm] r62045 - /llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp

[Nicolas Geoffray]

Hi Chris,

Chris Lattner wrote:
> On Jan 14, 2009, at 8:01 AM, Nicolas Geoffray wrote:
>   
>> Hi Chris,
>>
>> Quick question folllowing this patch. Getting the size of a type is
>> platform dependent, and since InstCombine makes lots of uses of
>> getTypeSize, InstCombine is therefore platform dependent. Is this  
>> OK? I
>> know this patch did not introduce the platform-dependency, but I was
>> just curious if that's OK, and if LLVM considers transformation passes
>> strongly platform dependent.
>>     
>
> Hi Nicolas,
>
> In the current state of LLVM, this is definitely the right thing to do.
>
>   

OK.

> However, I don't think that the current state is really what we want.

Agree.

>    
> target data info is *optional* in a module.  It is always specified by  
> (e.g.) llvm-gcc, but I don't think that target-independent languages  
> like Java should have to specify it. 

Agree.

> In my ideal world, not  
> specifying a target data string would transparently turn off the  
> subset of optimizations that depend on target data info only (not all  
> instcombines).
>
> Pragmatically speaking, the way I would like to (someday) implement  
> this is to change "opt" (and friends) to only add TargetData to the  
> passmanager if the module's targetdata string is non-empty.  Doing  
> this today would break all passes that addRequired<TargetData>().   
> This means that before we can do this, we have to change these passes  
> (like instcombine) to not have a hard requirement on targetdata.
>
> IMO, mid-level optimizations that we want to run from "opt" or "llvm- 
> ld" should never call addRequired<TargetData>/ 
> getAnalysis<TargetData>.  Instead, they should call  
> getAnalysisToUpdate<TargetData>().  This method returns a pointer if  
> TargetData is around, or returns null if not.  That would mean that  
> passes (like instcombine) that want to use targetdata would (for  
> xforms that need it) check to see if the pointer is non-null, then try  
> to do the xform in question.
>
> Does this seem reasonable?
>   

Yes it does. VMKit outputs a portable .bc from Java or C#, and it's 
unfortunate opt is changing it to a non-portable .bc file.

Nicolas