Csaba Ringhofer resolved IMPALA-5801.
    Resolution: Done

IMPALA-5801: Clean up codegen GetType() interface

Several functions that return llvm::(Pointer)Type were renamed
to make them shorter or indicate their roles more clearly. Some
additional convenience functions were created to make some common
codegen tasks simpler:

- Get(Ptr)Type functions with string parameter are renamed to
- GetStruct(Ptr)Type template functions are created to make
  GetNamedType(MyStruct::LLVM_CLASS_NAME) calls simpler (some
  classes had LLVM_CLASS_NAME as private, these are changed to
- integer type convenience functions are renamed to indicate
  bit width instead of matching SQL type (e.g. int_type->i32_type)
- similar convenience functions were created for ptr to primitive
  types, int_ptr_type
- Get(Ptr)Type functions with ColumnType parameter are renamed
  to GetSlot(Ptr)Type
- GetIntConstant function is split to several functions depending
  on the type of the integer e.g. GetI32Constant

The renamed functions can be found in llvm-codegen.h/cc. Changes
in other files are mainly renamed calls with the same functionality.

No new tests are necessary, as no functionality was changed.

Change-Id: Ib146ca21af82023b0b460f813eae3435b4b2eb77
Reviewed-on: http://gerrit.cloudera.org:8080/9063
Reviewed-by: Tim Armstrong <tarmstr...@cloudera.com>
Tested-by: Impala Public Jenkins

> Clean up codegen GetType() interface
> ------------------------------------
>                 Key: IMPALA-5801
>                 URL: https://issues.apache.org/jira/browse/IMPALA-5801
>             Project: IMPALA
>          Issue Type: Improvement
>          Components: Backend
>            Reporter: Tim Armstrong
>            Assignee: Csaba Ringhofer
>            Priority: Minor
>              Labels: codegen
> The naming and usage of the two LlvmCodegen::GetType() methods is confusing. 
> They seem to serve multiple distinct purposes:
> * GetType(const string&) is simple enough - it looks up a predeclared type by 
> name.
> * GetType(ColumnType) returns the internal representation of a column type. 
> At some callsites it is used for this purpose. We should probably rename to 
> GetInternalRepresentation() or something like that.
> * GetType(ColumnType) is also used as a convenience function to get a 
> specific integer type, e.g. GetType(TYPE_INT). Those callsites should 
> probably just call codegen->int_type() instead of using the general-purpose 
> GetType() function.

This message was sent by Atlassian JIRA

Reply via email to