Re: [PATCH 02/10][v6] powerpc/Power7: detect load/store instructions

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 10/16/2013 01:55 PM, David Laight wrote:
>> Implement instr_is_load_store_2_06() to detect whether a given instruction
>> is one of the fixed-point or floating-point load/store instructions in the
>> POWER Instruction Set Architecture v2.06.
> ...

The op code encoding is dependent on the ISA version ? Does the basic load
and store instructions change with newer ISA versions ? BTW we have got a
newer version for the ISA "PowerISA_V2.07_PUBLIC.pdf" here at power.org

https://www.power.org/documentation/power-isa-version-2-07/

Does not sound like a good idea to analyse the instructions with functions
names which specify ISA version number. Besides, this function does not
belong to specific processor or platform. It has to be bit generic.
 
>> +int instr_is_load_store_2_06(const unsigned int *instr)
>> +{
>> +	unsigned int op, upper, lower;
>> +
>> +	op = instr_opcode(*instr);
>> +
>> +	if ((op >= 32 && op <= 58) || (op == 61 || op == 62))
>> +		return true;
>> +
>> +	if (op != 31)
>> +		return false;
>> +
>> +	upper = op >> 5;
>> +	lower = op & 0x1f;
>> +
>> +	/* Short circuit as many misses as we can */
>> +	if (lower < 3 || lower > 23)
>> +		return false;
>> +
>> +	if (lower == 3) {
>> +		if (upper >= 16)
>> +			return true;
>> +
>> +		return false;
>> +	}
>> +
>> +	if (lower == 7 || lower == 12)
>> +		return true;
>> +
>> +	if (lower >= 20) /* && lower <= 23 (implicit) */
>> +		return true;
>> +
>> +	return false;
>> +}
> 
> I can't help feeling the code could do with some comments about
> which actual instructions are selected where.

Yeah, I agree. At least which category of load-store instructions are
getting selected in each case.