Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/45820 )

Change subject: arch-x86: Work around a bug in g++ 6 and 7.
......................................................................

arch-x86: Work around a bug in g++ 6 and 7.

These versions of g++ don't handle parameter pack expansion correctly
when there is a parameter pack defined at the class level and then one
which is defined by the constructor itself. Even though it knows what
the outter parameter pack contains, it still re-assigns it to be empty
and puts all arguments into the later parameter pack.

To work around this problem, we will explicitly put the class level
parameters into a tuple, which we then have to go through extra
acrobatics to explode and pass into base class constructors.

That also means that in all subclasses, the arguments which go into the
tuple need to be wrapped in {}s to group them into constructor arguments
for the tuple.

Change-Id: I3139eebd7042b02f50862d88be5c940583a2a809
---
M src/arch/x86/insts/microldstop.hh
M src/arch/x86/insts/microop_args.hh
M src/arch/x86/insts/microspecop.hh
M src/arch/x86/isa/microops/fpop.isa
M src/arch/x86/isa/microops/limmop.isa
M src/arch/x86/isa/microops/mediaop.isa
M src/arch/x86/isa/microops/regop.isa
M src/arch/x86/isa/microops/seqop.isa
M src/arch/x86/isa/microops/specop.isa
9 files changed, 33 insertions(+), 16 deletions(-)



diff --git a/src/arch/x86/insts/microldstop.hh b/src/arch/x86/insts/microldstop.hh
index 067e5bd..8ce458b 100644
--- a/src/arch/x86/insts/microldstop.hh
+++ b/src/arch/x86/insts/microldstop.hh
@@ -39,6 +39,8 @@
 #ifndef __ARCH_X86_INSTS_MICROLDSTOP_HH__
 #define __ARCH_X86_INSTS_MICROLDSTOP_HH__

+#include <tuple>
+
 #include "arch/x86/insts/microop.hh"
 #include "arch/x86/insts/microop_args.hh"
 #include "arch/x86/ldstflags.hh"
@@ -89,7 +91,7 @@
             Request::FlagsType mem_flags, OpClass op_class) :
     InstOperands<MemOp, FoldedDataOp, AddrOp>(
             mach_inst, mnem, inst_mnem, set_flags, op_class,
-            _data, { _scale, _index, _base, _disp, _segment },
+            { _data, { _scale, _index, _base, _disp, _segment } },
             data_size, address_size, mem_flags | _segment.index)
     {}
 };
@@ -108,7 +110,7 @@
             Request::FlagsType mem_flags, OpClass op_class) :
     InstOperands<MemOp, FloatDataOp, AddrOp>(
             mach_inst, mnem, inst_mnem, set_flags, op_class,
-            _data, { _scale, _index, _base, _disp, _segment },
+            { _data, { _scale, _index, _base, _disp, _segment } },
             data_size, address_size, mem_flags | _segment.index)
     {}
 };
@@ -126,7 +128,7 @@
             Request::FlagsType mem_flags, OpClass op_class) :
     InstOperands<MemOp, AddrOp>(
             mach_inst, mnem, inst_mnem, set_flags, op_class,
-            { _scale, _index, _base, _disp, _segment },
+            { { _scale, _index, _base, _disp, _segment } },
             data_size, address_size, mem_flags | _segment.index)
     {}
 };
@@ -148,7 +150,7 @@
             Request::FlagsType mem_flags, OpClass op_class) :
     InstOperands<MemOp, FoldedDataLowOp, FoldedDataHiOp, AddrOp>(
             mach_inst, mnem, inst_mnem, set_flags, op_class,
-            data_low, data_hi, { _scale, _index, _base, _disp, _segment },
+ { data_low, data_hi, { _scale, _index, _base, _disp, _segment } },
             data_size, address_size, mem_flags | _segment.index)
     {}
 };
diff --git a/src/arch/x86/insts/microop_args.hh b/src/arch/x86/insts/microop_args.hh
index 3a9cb06..f810a74 100644
--- a/src/arch/x86/insts/microop_args.hh
+++ b/src/arch/x86/insts/microop_args.hh
@@ -31,6 +31,8 @@
 #include <cstdint>
 #include <sstream>
 #include <string>
+#include <tuple>
+#include <utility>

 #include "arch/x86/insts/static_inst.hh"
 #include "arch/x86/regs/int.hh"
@@ -338,13 +340,26 @@
 template <typename Base, typename ...Operands>
 class InstOperands : public Base, public Operands...
 {
+  private:
+    using ArgTuple = std::tuple<typename Operands::ArgType...>;
+
+    template <std::size_t ...I, typename ...CTorArgs>
+    InstOperands(std::index_sequence<I...>, ExtMachInst mach_inst,
+            const char *mnem, const char *inst_mnem, uint64_t set_flags,
+            OpClass op_class, GEM5_VAR_USED ArgTuple args,
+            CTorArgs... ctor_args) :
+ Base(mach_inst, mnem, inst_mnem, set_flags, op_class, ctor_args...),
+        Operands(this, std::get<I>(args))...
+    {}
+
   protected:
     template <typename ...CTorArgs>
     InstOperands(ExtMachInst mach_inst, const char *mnem,
             const char *inst_mnem, uint64_t set_flags, OpClass op_class,
-            typename Operands::ArgType... args, CTorArgs... ctor_args) :
- Base(mach_inst, mnem, inst_mnem, set_flags, op_class, ctor_args...),
-        Operands(this, args)...
+            ArgTuple args, CTorArgs... ctor_args) :
+        InstOperands(std::make_index_sequence<sizeof...(Operands)>{},
+                mach_inst, mnem, inst_mnem, set_flags, op_class,
+                std::move(args), ctor_args...)
     {}

     std::string
diff --git a/src/arch/x86/insts/microspecop.hh b/src/arch/x86/insts/microspecop.hh
index 3d2c631..ed78024 100644
--- a/src/arch/x86/insts/microspecop.hh
+++ b/src/arch/x86/insts/microspecop.hh
@@ -42,7 +42,7 @@
         InstOperands<X86MicroopBase>(mach_inst, "halt", inst_mnem,
                 set_flags | (1ULL << StaticInst::IsNonSpeculative) |
                             (1ULL << StaticInst::IsQuiesce),
-                No_OpClass)
+                No_OpClass, {})
     {}

     Fault
diff --git a/src/arch/x86/isa/microops/fpop.isa b/src/arch/x86/isa/microops/fpop.isa
index f727fdc..dc83362 100644
--- a/src/arch/x86/isa/microops/fpop.isa
+++ b/src/arch/x86/isa/microops/fpop.isa
@@ -92,7 +92,7 @@
             const char *inst_mnem, uint64_t set_flags,
             uint8_t data_size, int8_t _spm, Args... args) :
         %(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
-                %(op_class)s, args..., data_size, _spm)
+                %(op_class)s, { args... }, data_size, _spm)
     {
         %(set_reg_idx_arr)s;
         %(constructor)s;
diff --git a/src/arch/x86/isa/microops/limmop.isa b/src/arch/x86/isa/microops/limmop.isa
index b0444b8..c1312e6 100644
--- a/src/arch/x86/isa/microops/limmop.isa
+++ b/src/arch/x86/isa/microops/limmop.isa
@@ -64,7 +64,7 @@
                 uint64_t set_flags, Dest _dest,
                 uint64_t _imm, uint8_t data_size) :
         %(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
-                %(op_class)s, _dest, _imm, data_size, 0)
+                %(op_class)s, { _dest, _imm }, data_size, 0)
     {
         %(set_reg_idx_arr)s;
         %(constructor)s;
diff --git a/src/arch/x86/isa/microops/mediaop.isa b/src/arch/x86/isa/microops/mediaop.isa
index 3effff8..7577728 100644
--- a/src/arch/x86/isa/microops/mediaop.isa
+++ b/src/arch/x86/isa/microops/mediaop.isa
@@ -58,7 +58,7 @@
                 uint64_t set_flags, uint8_t src_size, uint8_t dest_size,
                 uint16_t _ext, Args... args) :
             %(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
-                    %(op_class)s, args..., src_size, dest_size, _ext)
+                    %(op_class)s, { args... }, src_size, dest_size, _ext)
         {
             %(set_reg_idx_arr)s;
             %(constructor)s;
diff --git a/src/arch/x86/isa/microops/regop.isa b/src/arch/x86/isa/microops/regop.isa
index 13b801c..0cca5b5 100644
--- a/src/arch/x86/isa/microops/regop.isa
+++ b/src/arch/x86/isa/microops/regop.isa
@@ -79,7 +79,7 @@
                 uint64_t set_flags, uint8_t data_size, uint16_t _ext,
                 Args... args) :
             %(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
-                    %(op_class)s, args..., data_size, _ext)
+                    %(op_class)s, { args... }, data_size, _ext)
         {
             %(set_reg_idx_arr)s;
             %(constructor)s;
@@ -102,7 +102,7 @@
                 uint64_t set_flags, uint8_t data_size, uint16_t _ext,
                 Args... args) :
             %(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
-                    %(op_class)s, args..., data_size, _ext)
+                    %(op_class)s, { args... }, data_size, _ext)
         {
             %(set_reg_idx_arr)s;
             %(constructor)s;
diff --git a/src/arch/x86/isa/microops/seqop.isa b/src/arch/x86/isa/microops/seqop.isa
index 7b52101..5eae7e6 100644
--- a/src/arch/x86/isa/microops/seqop.isa
+++ b/src/arch/x86/isa/microops/seqop.isa
@@ -43,7 +43,7 @@
         %(class_name)s(ExtMachInst mach_inst, const char *inst_mnem,
                 uint64_t set_flags, uint16_t _target, uint8_t _cc) :
             %(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
-                    %(op_class)s, _target, _cc)
+                    %(op_class)s, { _target }, _cc)
         {
             %(set_reg_idx_arr)s;
             %(constructor)s;
@@ -77,7 +77,7 @@
         %(class_name)s(ExtMachInst mach_inst, const char *inst_mnem,
                 uint64_t set_flags, uint8_t _cc) :
             %(base_class)s(mach_inst, "%(mnemonic)s", inst_mnem, set_flags,
-                    %(op_class)s, _cc)
+                    %(op_class)s, {}, _cc)
         {
             %(set_reg_idx_arr)s;
             %(constructor)s;
diff --git a/src/arch/x86/isa/microops/specop.isa b/src/arch/x86/isa/microops/specop.isa
index c5fe5bc..0a720ee 100644
--- a/src/arch/x86/isa/microops/specop.isa
+++ b/src/arch/x86/isa/microops/specop.isa
@@ -75,7 +75,7 @@
ExtMachInst mach_inst, const char *inst_mnem, uint64_t set_flags,
             Fault _fault, uint8_t _cc) :
%(base_class)s(mach_inst, "fault", inst_mnem, set_flags, No_OpClass,
-                _fault, _cc)
+                { _fault }, _cc)
     {
         %(set_reg_idx_arr)s;
         %(constructor)s;

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/45820
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I3139eebd7042b02f50862d88be5c940583a2a809
Gerrit-Change-Number: 45820
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to