This is an archive of the discontinued LLVM Phabricator instance.

Fix R0 use in PowerPC VSX store for FastIsel

Authored by sfantao on Mar 16 2015, 10:41 AM.



Hi all,

I observed that for some constructors, e.g:

struct specific_fpval {

double Val;
specific_fpval(double V) : Val(V) {}


the VSX store generated to store V into Val is not specifying any offset register causing %noreg to be generated and R0 to be emitted later on. The semantics of the VSX store (e.g. stdsdx) requires R0 to be used as base if we want zero to be used in the computation of the effective address instead of the content of R0. This patch checks if no index register was generated and forces R0 to be used as base address.

This was causing clang auto-compilation to fail in Debug mode. I understand that VSX support is still work under progress, but I decided to contribute this patch in the event this is something no one else was aware. I included a regression in a separate file.


Diff Detail

Event Timeline

sfantao updated this revision to Diff 22031.Mar 16 2015, 10:41 AM
sfantao retitled this revision from to Fix R0 use in PowerPC VSX store for FastIsel.
sfantao updated this object.
sfantao edited the test plan for this revision. (Show Details)
sfantao added reviewers: hfinkel, wschmidt.
sfantao added a subscriber: Unknown Object (MLST).
hfinkel edited edge metadata.Mar 16 2015, 10:53 AM

There are various places where we call:

MRI.setRegClass(SomeReg, &PPC::GPRC_and_GPRC_NOR0RegClass);

to adjust for this issue. Does calling that on IndexReg help? I suppose I don't understand exactly what's going on here? Is it generally legal to call MIBuilder::addReg with an invalid register operand?

Before this fix, there were instructions being generated like this:

STXSDX %F1<kill>, %X3<kill>, %noreg, %RM<imp-use>

The code seems be prepared to handle %noreg. R0 was being emitted for %noreg. The problem here is not the register class but the order of the operands. If FastISel were emitting something like:

STXSDX %F1<kill>, %X3<kill>, %ZERO8, %RM<imp-use>

we would still have a problem, as the content of R0 would be taken to compute the effective address of the store. If we have:

STXSDX %F1<kill>, %ZERO8, %X3<kill>, %RM<imp-use>

this works fine as R0 used as based address makes 0 to be used in the computation of the effective address instead of the value contained in R0. We are doing something similar for when UseOffset is set, the immediate has to come before the base register in order to get the right semantics.


hfinkel accepted this revision.Mar 17 2015, 5:07 AM
hfinkel edited edge metadata.

LGTM, thanks!

Can you also look into where the %noreg -> r0 mapping happened? Are we missing an assert somewhere in the backend? As you're well aware, we don't really have a general null register in PPC, and I suspect that anywhere we have a one in an instruction is a bug.

This revision is now accepted and ready to land.Mar 17 2015, 5:07 AM

Committed in r232486!

I will follow up on the %noreg emission in a separate thread.


sfantao closed this revision.Mar 17 2015, 10:41 AM