[PATCH v8] PowerPC: Support for Elliptic Curve Cryptography Instructions (RFC02669)
Surya Kumari Jangala
jskumari@linux.ibm.com
Sun Feb 1 09:21:33 GMT 2026
On 31/01/26 9:25 pm, Abhay Kandpal wrote:
> Hi Surya
>
> On 30/01/26 16:30, Surya Kumari Jangala wrote:
>> Hi,
>>
>> On 28/01/26 12:07 am, Abhay Kandpal wrote:
>>> diff --git a/opcodes/ppc-opc.c b/opcodes/ppc-opc.c
>>> index dfc6734e027..0a69a111f27 100644
>>> --- a/opcodes/ppc-opc.c
>>> +++ b/opcodes/ppc-opc.c
>>> @@ -2359,6 +2359,31 @@ extract_xab6 (uint64_t insn,
>>> return xa6;
>>> }
>>> +/* The S field (bits 21-23) in vector multiply multiply XX3 form instruction. */
>>> +
>>> +static uint64_t
>>> +insert_s3 (uint64_t insn,
>>> + int64_t value,
>>> + ppc_cpu_t dialect ATTRIBUTE_UNUSED,
>>> + const char **errmsg)
>>> +{
>>> + if (value < 0 || value > 6)
>>> + *errmsg = _("invalid S value (must be 0 - 6)");
>>> + return (insn | ((value & 0x7) << 8));
>> Remove the extra redundant parentheses.
>> Just insn | ((value & 0x7) << 8) is enough.
>>
>>> +}
>>> +
>>> +static int64_t
>>> +extract_s3 (uint64_t insn,
>>> + ppc_cpu_t dialect ATTRIBUTE_UNUSED,
>>> + int *invalid)
>>> +{
>>> + int64_t value = (insn >> 8) & 0x7;
>>> +
>>> + if (value == 7)
>>> + *invalid = 1;
>>> + return value;
>>> +}
>>> +
>>> /* The XC field in an XX4 form instruction. This is split. */
>>> static uint64_t
>>> @@ -3729,8 +3754,14 @@ const struct powerpc_operand powerpc_operands[] =
>>> #define MMMM SIX
>>> { 0xf, 11, NULL, NULL, 0 },
>>> + /* The P bit in vector scaled multiply-sum XX4 form prefix instruction. */
>>> +#define PSSUMEXT SIX + 1
>>> + { 0x1, 4, NULL, NULL, 0 },
>>> +
>>> + /* The S1 bit in a vector multiply multiply XX3 form instruction (bit 22). */
>>> +#define S1EXP PSSUMEXT + 1
>>> /* The PS field in a VX form instruction. */
>>> -#define PS SIX + 1
>>> +#define PS PSSUMEXT + 1
>> This should be #define PS S1EXP
>
> Though the current definition is functionally correct,
> I’ll update it to use|#define PS S1EXP| to align with the existing coding style.
A similar error in this patch was pointed out in an earlier review mail.
When that was fixed, this too should have been fixed.
And it is not right to say that this is just a coding style. This is more
than a coding style. Following this will ensure that fewer mistakes due to
oversight will occur.
If ever a new macro is added above S1EXP, then a change is required to be made
only in the definition of S1EXP; the other macros need not be touched.
This ensures that one does not commit mistakes due to oversight.
>
>>
>>> { 0x1, 9, NULL, NULL, 0 },
>>> /* The SH field in a vector shift double by bit immediate instruction. */
>>> @@ -3778,6 +3809,10 @@ const struct powerpc_operand powerpc_operands[] =
>>> /* PowerPC paired singles extensions. */
>>> /* W bit in the pair singles instructions for x type instructions. */
>>> #define PSWM WS + 1
>>> + /* The P bit in scaled multiply-sum XX3 form instructions (bit 21). */
>> s/scaled/vector scaled
>>
>>> +#define PSSUM PSWM
>>> + /* The S0 bit bit in a vector multiply multiply XX3 form instruction (bit 21). */
>> Remove the second mention of the word 'bit'.
>>
>>> +#define S0EXP PSWM
>>> /* The BO16 field in a BD8 form instruction. */
>>> #define BO16 PSWM
>>> /* The pst field in a SVRM form instruction. */
>>> @@ -3794,8 +3829,13 @@ const struct powerpc_operand powerpc_operands[] =
>>> #define PSQM PSQ + 1
>>> { 0x7, 7, 0, 0, PPC_OPERAND_GQR },
>>> + /* The S field (bits 21-23) in vector multiply multiply XX3 form
>>> + as an arithmetic function. */
>>> +#define SFUNC PSQM + 1
>>> + { 0x7, 8, insert_s3, extract_s3, 0 },
>>> +
>>> /* Smaller D field for quantization in the pair singles instructions. */
>>> -#define PSD PSQM + 1
>>> +#define PSD SFUNC + 1
>>> { 0xfff, 0, 0, 0, PPC_OPERAND_PARENS | PPC_OPERAND_SIGNED },
>>> /* The L field in an mtmsrd or A form instruction or R or W in an
>>> @@ -4026,6 +4066,8 @@ const struct powerpc_operand powerpc_operands[] =
>>> #define ms vs + 1
>>> #define yx ms
>>> + /* The S2 bit in a vector multiply multiply XX3 form instruction (23). */
>> s/23/bit 23
>>
>>> +#define S2EXP ms
>>> /* The P field in Galois Field XX3 form instruction. */
>>> #define PGF1 yx
>>> { 0x1, 8, NULL, NULL, 0 },
>>> @@ -4105,6 +4147,7 @@ const unsigned int num_powerpc_operands = ARRAY_SIZE (powerpc_operands);
>>> #define P_XX4_MASK (PREFIX_MASK | XX4_MASK)
>>> #define P_UXX4_MASK (P_XX4_MASK & ~(7ULL << 32))
>>> #define P_U8XX4_MASK (P_XX4_MASK & ~(0xffULL << 32))
>>> +#define P_XX4EXT_MASK (PREFIX_MASK | XX4EXT (0x3f, 0x1))
>>> /* MMIRR:XX3-form 8-byte outer product instructions. */
>>> #define P_GER_MASK ((-1ULL << 40) | XX3ACC_MASK)
>>> @@ -4597,6 +4640,9 @@ const unsigned int num_powerpc_operands = ARRAY_SIZE (powerpc_operands);
>>> /* An XX4 form instruction. */
>>> #define XX4(op, xop) (OP (op) | ((((uint64_t)(xop)) & 0x3) << 4))
>>> +/* An XX4 form instruction with 1 bit carry extended. */
>> This comment does not make any sense. You can rewrite as:
>> "An XX4 form Vector Scaled Multiply-Sum instruction"
>>
>>> +#define XX4EXT(op, xop) (OP (op) | ((((uint64_t)(xop)) & 0x1) << 5))
>> And rewrite XX4EXT as VMULSUM, or VMSOP (see VSOP).
>>
>> Rewrite P_XX4EXT_MASK as P_VMS_MASK.
>>
>> -Surya
>
> I will renamed it but just for your information. These macros(|XX4EXT| /|P_XX4EXT_MAS|) were already renamed
> in v2 of the patch based on the feedback and consensus at that time.
Yeah, but the EXT in XX4EXT doesn't make sense.
>
>>
>>> +
>>> /* A Z form instruction. */
>>> #define Z(op, xop) (OP (op) | ((((uint64_t)(xop)) & 0x1ff) << 1))
>>> @@ -4677,10 +4723,11 @@ const unsigned int num_powerpc_operands = ARRAY_SIZE (powerpc_operands);
>>> /* An X_MASK with two dense math register. */
>>> #define XDMRDMR_MASK (X_MASK | RA_MASK | (3 << 21) | (3 << 11))
>>> -/* The mask for an XX3 form instruction with the DM or SHW bits
>>> +/* The mask for an XX3 form instruction with the S1, S2, DM or SHW bits
>>> specified. */
>>> #define XX3DM_MASK (XX3 (0x3f, 0x1f) | (1 << 10))
>>> #define XX3SHW_MASK XX3DM_MASK
>>> +#define XX3MADD_MASK XX3DM_MASK
>>> /* The masks for X* form instructions with an ACC/DMR register. */
>>> #define XX2ACC_MASK (XX2 (0x3f, 0x1ff) | (3 << 21) | 1)
>>> @@ -4700,6 +4747,12 @@ const unsigned int num_powerpc_operands = ARRAY_SIZE (powerpc_operands);
>>> /* The masks for XX3 GF instructions with P bit. */
>>> #define XX3GF_MASK (XX3 (0x3f, 0xff) & ~(1 << 8))
>>> +/* The masks for VSX multiply XX3 instructions with scale bits. */
>>> +#define XX3MUL_MASK (XX3 (0x3f, 0x1f))
>>> +
>>> +/* The masks for VSX multiplty-sum XX3 instructions with p bits. */
>>> +#define XX3SUM_MASK (XX3 (0x3f, 0x7f))
>>> +
>>> /* The mask for an XX4 form instruction. */
>>> #define XX4_MASK XX4 (0x3f, 0x3)
>>> @@ -9202,6 +9255,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"dqua", ZRC(59,3,0), Z2_MASK, POWER6, PPCVLE, {FRT,FRA,FRB,RMC}},
>>> {"dqua.", ZRC(59,3,1), Z2_MASK, POWER6, PPCVLE, {FRT,FRA,FRB,RMC}},
>>> +{"xxmulmul", XX3(59,1), XX3MUL_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6, SFUNC}},
>>> {"dmxvi8ger4pp",XX3(59,2), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvi8ger4pp", XX3(59,2), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"dmxvi8ger4", XX3(59,3), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> @@ -9250,6 +9304,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"drrnd", ZRC(59,35,0), Z2_MASK, POWER6, PPCVLE, {FRT, FRA, FRB, RMC}},
>>> {"drrnd.", ZRC(59,35,1), Z2_MASK, POWER6, PPCVLE, {FRT, FRA, FRB, RMC}},
>>> +{"xxmulmulhiadd", XX3(59,9), XX3MUL_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6, S0EXP, S1EXP, S2EXP}},
>>> {"dmxvi8gerx4pp", XX3(59,10), XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> {"dmxvi8gerx4", XX3(59,11), XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> @@ -9259,6 +9314,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"dquai", ZRC(59,67,0), Z2_MASK, POWER6, PPCVLE, {TE, FRT,FRB,RMC}},
>>> {"dquai.", ZRC(59,67,1), Z2_MASK, POWER6, PPCVLE, {TE, FRT,FRB,RMC}},
>>> +{"xxmulmulloadd",XX3(59,17), XX3MADD_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6, S1EXP, S2EXP}},
>>> {"dmxvf16ger2pp",XX3(59,18), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvf16ger2pp", XX3(59,18), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"dmxvf16ger2", XX3(59,19), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> @@ -9270,6 +9326,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"drintx", ZRC(59,99,0), Z2_MASK, POWER6, PPCVLE, {R, FRT, FRB, RMC}},
>>> {"drintx.", ZRC(59,99,1), Z2_MASK, POWER6, PPCVLE, {R, FRT, FRB, RMC}},
>>> +{"xxssumudm", XX3(59,25), XX3SUM_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6, PSSUM}},
>>> {"dmxvf32gerpp",XX3(59,26), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvf32gerpp", XX3(59,26), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"dmxvf32ger", XX3(59,27), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> @@ -9301,6 +9358,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"drintn", ZRC(59,227,0), Z2_MASK, POWER6, PPCVLE, {R, FRT, FRB, RMC}},
>>> {"drintn.", ZRC(59,227,1), Z2_MASK, POWER6, PPCVLE, {R, FRT, FRB, RMC}},
>>> +{"xxssumudmc", XX3(59,57), XX3SUM_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6, PSSUM}},
>>> {"dmxvf64gerpp",XX3(59,58), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6ap, XB6a}},
>>> {"xvf64gerpp", XX3(59,58), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6ap, XB6a}},
>>> {"dmxvf64ger", XX3(59,59), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6ap, XB6a}},
>>> @@ -9329,21 +9387,26 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"dxex", XRC(59,354,0), X_MASK, POWER6, PPCVLE, {FRT, FRB}},
>>> {"dxex.", XRC(59,354,1), X_MASK, POWER6, PPCVLE, {FRT, FRB}},
>>> +{"xsmerge2t3uqm", XX3(59,89), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvf32gernp", XX3(59,90), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvf32gernp", XX3(59,90), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"dmxvbf16gerx2", XX3(59,91), XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> +{"xsaddadduqm", XX3(59,96), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvi8gerx4spp",XX3(59,98), XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> {"dmxvi8ger4spp", XX3(59,99), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvi8ger4spp", XX3(59,99), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> +{"xsaddaddsuqm", XX3(59,104), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvi16ger2pp", XX3(59,107), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvi16ger2pp", XX3(59,107), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> +{"xsaddsubuqm", XX3(59,112), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvbf16ger2np",XX3(59,114), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvbf16ger2np", XX3(59,114), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"dmxvbf16gerx2np",XX3(59,115), XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> +{"xsmerge3t1uqm", XX3(59,121), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvf64gernp", XX3(59,122), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6ap, XB6a}},
>>> {"xvf64gernp", XX3(59,122), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6ap, XB6a}},
>>> @@ -9353,6 +9416,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"ddiv", XRC(59,546,0), X_MASK, POWER6, PPCVLE, {FRT, FRA, FRB}},
>>> {"ddiv.", XRC(59,546,1), X_MASK, POWER6, PPCVLE, {FRT, FRA, FRB}},
>>> +{"xsrebase2t1uqm",XX3(59,145), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvf16ger2pn", XX3(59,146), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvf16ger2pn", XX3(59,146), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"dmxvf16gerx2pn",XX3(59,147), XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> @@ -9365,6 +9429,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"dtstsf", X(59,674), X_MASK, POWER6, PPCVLE, {BF, FRA, FRB}},
>>> {"dtstsfi", X(59,675), X_MASK|1<<22,POWER9, PPCVLE, {BF, UIM6, FRB}},
>>> +{"xsrebase2t2uqm",XX3(59,177), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvbf16ger2pn",XX3(59,178), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvbf16ger2pn", XX3(59,178), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"dmxvbf16gerx2pn", XX3(59,179),XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> @@ -9375,6 +9440,8 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"drsp", XRC(59,770,0), X_MASK, POWER6, PPCVLE, {FRT, FRB}},
>>> {"drsp.", XRC(59,770,1), X_MASK, POWER6, PPCVLE, {FRT, FRB}},
>>> +{"xsrebase3t3uqm",XX3(59,195), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> +
>>> {"dcffix", XRC(59,802,0), X_MASK|FRA_MASK, POWER7, PPCVLE, {FRT, FRB}},
>>> {"dcffix.", XRC(59,802,1), X_MASK|FRA_MASK, POWER7, PPCVLE, {FRT, FRB}},
>>> @@ -9383,6 +9450,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"denbcd", XRC(59,834,0), X_MASK, POWER6, PPCVLE, {S, FRT, FRB}},
>>> {"denbcd.", XRC(59,834,1), X_MASK, POWER6, PPCVLE, {S, FRT, FRB}},
>>> +{"xsrebase2t3uqm",XX3(59,209), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvf16ger2nn", XX3(59,210), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvf16ger2nn", XX3(59,210), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> @@ -9392,17 +9460,22 @@ const struct powerpc_opcode powerpc_opcodes[] = {
>>> {"diex", XRC(59,866,0), X_MASK, POWER6, PPCVLE, {FRT, FRA, FRB}},
>>> {"diex.", XRC(59,866,1), X_MASK, POWER6, PPCVLE, {FRT, FRA, FRB}},
>>> +{"xsrebase2t4uqm",XX3(59,217), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvf32gernn",XX3(59,218), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"xvf32gernn", XX3(59,218), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> -{"dmxvbf16gerx2nn", XX3(59,234),XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> -
>>> -{"dmxvbf16ger2nn",XX3(59,242), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> -{"xvbf16ger2nn", XX3(59,242), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> +{"xsaddsubsuqm", XX3(59,224), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> +{"xsmerge2t1uqm", XX3(59,232), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> +{"dmxvbf16gerx2nn",XX3(59,234), XX3GERX_MASK, FUTURE, PPCVLE, {DMR, XA5p, XB6}},
>>> +{"xsmerge2t2uqm", XX3(59,240), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> +{"xsrebase3t1uqm", XX3(59,241), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> +{"dmxvbf16ger2nn", XX3(59,242), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> +{"xvbf16ger2nn", XX3(59,242), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6a, XB6a}},
>>> {"fcfidus", XRC(59,974,0), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT, FRB}},
>>> {"fcfidus.", XRC(59,974,1), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT, FRB}},
>>> +{"xsrebase3t2uqm",XX3(59,249), XX3_MASK, FUTURE, PPCVLE, {XT6, XA6, XB6}},
>>> {"dmxvf64gernn",XX3(59,250), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6ap, XB6a}},
>>> {"xvf64gernn", XX3(59,250), XX3ACC_MASK, POWER10, PPCVLE, {ACC, XA6ap, XB6a}},
>>> @@ -10003,6 +10076,7 @@ const struct powerpc_opcode prefix_opcodes[] = {
>>> {"xxblendvd", P8RR|XX4(33,3), P_XX4_MASK, POWER10, 0, {XT6, XA6, XB6, XC6}},
>>> {"xxpermx", P8RR|XX4(34,0), P_UXX4_MASK, POWER10, 0, {XT6, XA6, XB6, XC6, UIM3}},
>>> {"xxeval", P8RR|XX4(34,1), P_U8XX4_MASK, POWER10, 0, {XT6, XA6, XB6, XC6, UIM8}},
>>> +{"xxssumudmcext", P8RR|XX4EXT(34,1), P_XX4EXT_MASK, FUTURE, 0, {XT6, XA6, XB6, XC6, PSSUMEXT}},
>>> {"plbz", PMLS|OP(34), P_D_MASK, POWER10, 0, {RT, D34, PRA0, PCREL}},
>>> {"pstw", PMLS|OP(36), P_D_MASK, POWER10, 0, {RS, D34, PRA0, PCREL}},
>>> {"pstb", PMLS|OP(38), P_D_MASK, POWER10, 0, {RS, D34, PRA0, PCREL}},
>
> More generally, several of the comments relate to naming or formatting choices that have been present since earlier versions of the patch.
> It would be helpful if such preferences could be consolidated earlier in the review process,
> so they can be addressed together and avoid repeated rework across multiple revisions.
As discussed earlier, this is a long patch with several changes required and
it is easier for both me and you if the changes are tackled
in chunks. Easy for me so that I can check if the reworked patch has all
the comments addressed. It is easier for you too to ensure all
comments are addressed. Do note that a prior review of this patch had only
few comments given but the reworked patch had one comment missing. If this
is the case for few comments, imagine the situation when several comments are given!
The best way for speeding up patch reviews is to ensure that all the
formatting is correct, double check all review comments are addressed, ensure
that the same/similar errors are not repeated, go through the rest of the code
to ensure that a similar error is not present, ensure that comments and variable
names are meaningful (you can go thru existing code to get a hang of how to come
up with variable names and how to write comments).
-Surya
>
> BR
> Abhay
>
More information about the Binutils
mailing list