Balazs Hevele has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24132 )

Change subject: IMPALA-14851: Codegen GroupingAggregator::CopyGroupingValues
......................................................................


Patch Set 11:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/24132/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24132/10//COMMIT_MSG@18
PS10, Line 18: AGGREGATION_NODE Avg Time changed from 743.281ms to 659.414ms.
> Can you add the difference in the profile for some codegen metrics for affe
Done


http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/codegen/llvm-codegen.h
File be/src/codegen/llvm-codegen.h:

http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/codegen/llvm-codegen.h@634
PS10, Line 634:   /// Codegens IR to call the function corresponding to 
'ir_type' with argument 'args'
              :   /// and returns the value.
              :   /// 'name' is an optional IR name for the returned value.
              :   /// If the function returns void, 'name' has to be left empty.
              :   llvm::Value* CodegenCallFunction(LlvmBuilder* builder, 
IRFunction::Type ir_type,
              :       llvm::ArrayRef<llvm::Value*> args, const char* name = "");
              :
              :   /// If there are more than this num
> The difference between these functions seem to be that the new function has
The reason for this is that LLVM outputs an error if a name is given for the 
result of a call to a function returning void.
I pulled the functions together with the default value "" ( which is the 
default for llvm::IRBuilder::CreateCall's name argument as well), with some 
explanatory comments.


http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/exec/grouping-aggregator.cc
File be/src/exec/grouping-aggregator.cc:

http://gerrit.cloudera.org:8080/#/c/24132/10/be/src/exec/grouping-aggregator.cc@1284
PS10, Line 1284: //   call void 
@_ZN6impala18GroupingAggregator30CopyGroupingValuesFixedLenSlot...
> just curious, when do we get this output? I assumed that we'll never see IR
This is the result from LlvmCodeGen::Print.
Apparently we do see calls to IR_ALWAYS_INLINE functions here (before 
optimization).



--
To view, visit http://gerrit.cloudera.org:8080/24132
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia2c54a5745ba05a926795318d3b074fde6d0c00a
Gerrit-Change-Number: 24132
Gerrit-PatchSet: 11
Gerrit-Owner: Balazs Hevele <[email protected]>
Gerrit-Reviewer: Balazs Hevele <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Mon, 15 Jun 2026 10:44:04 +0000
Gerrit-HasComments: Yes

Reply via email to