Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1131)

Unified Diff: src/IceTargetLowering.h

Issue 1683243003: ARM32 Vector lowering - scalarize select (Closed) Base URL: https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Patch Set: Enable test_select crosstest Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: src/IceTargetLowering.h
diff --git a/src/IceTargetLowering.h b/src/IceTargetLowering.h
index 3477f709b2ef0ff60a6693d8625a88e125eb7eca..663dfc56ed1d6fc2c60a450bdcb171df2162a513 100644
--- a/src/IceTargetLowering.h
+++ b/src/IceTargetLowering.h
@@ -471,71 +471,34 @@ protected:
///
/// MakeInstruction is a function-like object with signature
/// (Variable *Dest, Variable *Src0, Variable *Src1) -> Instr *.
John 2016/02/11 15:02:41 out of date comment... that's why I would encoura
- template <typename F>
- void scalarizeInstruction(Variable *Dest, Operand *Src0, Operand *Src1,
- F &&MakeInstruction) {
John 2016/02/11 15:02:41 I didn't see this before, but I am not a fan of th
+ template <typename F, typename... Operands>
+ void scalarizeInstruction(Variable *Dest, F &&MakeInstruction,
John 2016/02/11 15:02:41 MakeInstruction is a parameter, hence it is named
Eric Holk 2016/02/11 19:17:23 That sounds alright to me. I went ahead and made t
+ Operands *... Srcs) {
const Type DestTy = Dest->getType();
assert(isVectorType(DestTy));
const Type DestElementTy = typeElementType(DestTy);
const SizeT NumElements = typeNumElements(DestTy);
- const Type Src0ElementTy = typeElementType(Src0->getType());
- const Type Src1ElementTy = typeElementType(Src1->getType());
-
- assert(NumElements == typeNumElements(Src0->getType()));
- assert(NumElements == typeNumElements(Src1->getType()));
Variable *T = Func->makeVariable(DestTy);
Context.insert<InstFakeDef>(T);
- for (SizeT I = 0; I < NumElements; ++I) {
- Constant *Index = Ctx->getConstantInt32(I);
-
- // Extract the next two inputs.
- Variable *Op0 = Func->makeVariable(Src0ElementTy);
- Context.insert<InstExtractElement>(Op0, Src0, Index);
- Variable *Op1 = Func->makeVariable(Src1ElementTy);
- Context.insert<InstExtractElement>(Op1, Src1, Index);
-
- // Perform the operation as a scalar operation.
- Variable *Res = Func->makeVariable(DestElementTy);
- auto Arith = MakeInstruction(Res, Op0, Op1);
- // We might have created an operation that needed a helper call.
- genTargetHelperCallFor(Arith);
- // Insert the result into position.
- Variable *DestT = Func->makeVariable(DestTy);
- Context.insert<InstInsertElement>(DestT, T, Res, Index);
- T = DestT;
- }
- Context.insert<InstAssign>(Dest, T);
- }
-
- template <typename F>
- void scalarizeUnaryInstruction(Variable *Dest, Operand *Src0,
- F &&MakeInstruction) {
- const Type DestTy = Dest->getType();
- assert(isVectorType(DestTy));
- const Type DestElementTy = typeElementType(DestTy);
- const SizeT NumElements = typeNumElements(DestTy);
- const Type Src0ElementTy = typeElementType(Src0->getType());
-
- assert(NumElements == typeNumElements(Src0->getType()));
-
- Variable *T = Func->makeVariable(DestTy);
- Context.insert<InstFakeDef>(T);
for (SizeT I = 0; I < NumElements; ++I) {
- Constant *Index = Ctx->getConstantInt32(I);
+ auto *Index = Ctx->getConstantInt32(I);
+
+ auto MakeExtract = [&](Operand *Src) {
John 2016/02/11 15:02:41 please, no default capture -- capture what you nee
Jim Stichnoth 2016/02/11 15:26:07 Check whether the experts (hi John!) are OK with t
Eric Holk 2016/02/11 19:17:23 Done.
+ assert(typeNumElements(Src->getType()) == NumElements);
- // Extract the next two inputs.
- Variable *Op0 = Func->makeVariable(Src0ElementTy);
- Context.insert<InstExtractElement>(Op0, Src0, Index);
+ const auto ElementTy = typeElementType(Src->getType());
+ Variable *Op = Func->makeVariable(ElementTy);
John 2016/02/11 15:02:41 auto here?
John 2016/02/11 15:02:41 why did you use makeVariable instead of makeReg?
Eric Holk 2016/02/11 19:17:23 makeReg is defined in TargetARM32, and this is in
Eric Holk 2016/02/11 19:17:23 Done.
+ Context.insert<InstExtractElement>(Op, Src, Index);
+ return Op;
+ };
// Perform the operation as a scalar operation.
Variable *Res = Func->makeVariable(DestElementTy);
- auto Arith = MakeInstruction(Res, Op0);
- // We might have created an operation that needed a helper call.
+ auto *Arith = MakeInstruction(Res, MakeExtract(Srcs)...);
John 2016/02/11 15:02:41 I just noticed that MakeInstruction is misleading.
Eric Holk 2016/02/11 19:17:23 Done.
genTargetHelperCallFor(Arith);
John 2016/02/11 15:02:41 Calling this method here is bad -- really bad. Th
Eric Holk 2016/02/11 19:17:23 Scalarize is called by genTargetHelperCallsFor, so
- // Insert the result into position.
Variable *DestT = Func->makeVariable(DestTy);
Context.insert<InstInsertElement>(DestT, T, Res, Index);
T = DestT;

Powered by Google App Engine
This is Rietveld 408576698