[llvm-commits] [PATCH]Convert vmov instructions between GPR and S registers for Cortex-A9

Anton Korobeynikov anton at korobeynikov.info
Sat Aug 11 04:37:05 PDT 2012


Silviu,

> I couldn't find any tabs in the patch. I did add a switch statement
> which changed the indentation for some code. I've fixed the other
> style issues.
Sorry, I misread the patch... Generally patch is ok, however I'd
suggest addressing the following issues:

1. inside getExecutionDomain() make a common if (CortexA9) switch.
Also please put the comment why the change is profitable on A9 only.
2. I really do not like stuff like this:
+      DReg = ARM::D0 + ((DstReg - ARM::S0) >> 1);
Here you're explicitly depending on particular encoding of enums, or,
rather mapping enum value => register number.
This asks for really weird and subtle bugs in the future when enums
e.g. will be changed. Will you please some other way to go from S
register to corresponding D register + lane? I believe there was some
patch about inline asm %H modifier provided recently which required
the same stuff - you might see how the problem was resolved there.

-- 
With best regards, Anton Korobeynikov
Faculty of Mathematics and Mechanics, Saint Petersburg State University



More information about the llvm-commits mailing list