This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Expand compare instructions to equivalent GPR code sequences.
AbandonedPublic

Authored by jtony on Feb 28 2017, 12:08 PM.

Details

Summary

The patterns (equal, equal to zero, great or equal to zero, signed great than zero, signed less or equal than, signed less than) that used to generate cmpw which sets CR bits are replaced by specific GPR code sequences that are outlined in the Compiler Writer's Guide (Appendix D). This will help us reduce the number of CR logical operations used.

This initial patch only handles those patterns that could be implemented in PPCInstrInfo.td. More patterns will be implemented in PPCISelDAGToDAG.cpp file in the next patch because of the limitation of table-gen (cannot handle multiple outs).

Note: that some of the test cases are added to test already existing patterns, that's why we have more test cases than patterns.

Also, the SETEQ pattern implemented here in the .td file could also be done in PPCISelDAGToDAG.cpp.

Diff Detail

Event Timeline

jtony created this revision.Feb 28 2017, 12:08 PM
inouehrs edited edge metadata.EditedMar 2 2017, 9:40 PM

Comparison is often used with bit operators, like an example below.
In this example, as long as I test, the transformation is applied only when POS is 0; cmpd + isel are used for cases with POS > 0.
I feel it is nice if we can support comparisons used with bit operators.

const int POS = 1;
long set_flag(long a, long b, long flags) {
	flags |= ((a == b) << POS);
	return flags;
}
hfinkel added inline comments.Mar 3 2017, 9:47 AM
lib/Target/PowerPC/PPCInstrInfo.td
3162

Please indent this by one more space so the O is under the C.

3166

Indent by one.

3174

Please indent for the P to be under the S.

3267

// This happens frequently because instruction selection will concert SETLE 0 to SETLT 1.

(abbreviating instruction selection as ISEL is confusing in this context because we also have an ISEL instruction).

jtony added a comment.Mar 3 2017, 2:52 PM

Comparison is often used with bit operators, like an example below.
In this example, as long as I test, the transformation is applied only when POS is 0; cmpd + isel are used for cases with POS > 0.
I feel it is nice if we can support comparisons used with bit operators.

const int POS = 1;
long set_flag(long a, long b, long flags) {
	flags |= ((a == b) << POS);
	return flags;
}

Hi Hiroshi, that happens because current the SETEQ is implemented in the PPCInstrInfo.td file, which is downstream of DAG2DAG slection, I checked the debug info for the test case you provided here. The zero_extend(setcc) pattern is got converted to select_cc before it has a chance to use the pattern that I added in td file. One possible solution is do that in the DAG2DAG stage like all the other patterns in the CWG, this is a similar issue Nemanja and I have encountered for these pattern in D.2 of CWG. And there is also a new commit from trunk changes the conversion behavior from setcc to select_cc. I haven't tried what is the influence of that commit so I am not sure whether we should move this implementation here to the PPCISelDAG2DAG.cpp file or not. I will let Kit and Nemanja decide.

jtony marked 4 inline comments as done.Mar 3 2017, 2:58 PM
jtony updated this revision to Diff 90542.Mar 3 2017, 3:08 PM

Resolve Hal's comments.

nemanjai edited edge metadata.Mar 3 2017, 11:23 PM

Comparison is often used with bit operators, like an example below.
In this example, as long as I test, the transformation is applied only when POS is 0; cmpd + isel are used for cases with POS > 0.
I feel it is nice if we can support comparisons used with bit operators.

const int POS = 1;
long set_flag(long a, long b, long flags) {
	flags |= ((a == b) << POS);
	return flags;
}

Hi Hiroshi, that happens because current the SETEQ is implemented in the PPCInstrInfo.td file, which is downstream of DAG2DAG slection, I checked the debug info for the test case you provided here. The zero_extend(setcc) pattern is got converted to select_cc before it has a chance to use the pattern that I added in td file. One possible solution is do that in the DAG2DAG stage like all the other patterns in the CWG, this is a similar issue Nemanja and I have encountered for these pattern in D.2 of CWG. And there is also a new commit from trunk changes the conversion behavior from setcc to select_cc. I haven't tried what is the influence of that commit so I am not sure whether we should move this implementation here to the PPCISelDAG2DAG.cpp file or not. I will let Kit and Nemanja decide.

This will be handled in a subsequent patch.

jtony updated this revision to Diff 92133.Mar 17 2017, 5:51 AM

Update the test cases according to the new convention. 16 test cases file added for SETCC EQ.

jtony abandoned this revision.Mar 22 2017, 6:27 AM

The SETCC EQ pattern implemented here are moved to PPCISelDAGToDAG.cpp file in a following up patch, abandon this one.