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

[Chris Lattner]

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.

However, I don't think that the current state is really what we want.   
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.  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?

-Chris