[llvm-commits] [PATCH] arm BFI instruction!

[Renato Golin]

On 01/17/11 18:13, Bruno Cardoso Lopes wrote:
> ping,
> I have more patches that fix other encoding and asm parsing issues for
> other instructions, should I send all for revision or is it better to
> commit directly (with testcases) and leave then for post-commit
> review?

Hi Bruno,

Sorry for the delay. I have a few questions...

First, can you describe the difference between the current model and the
new (alternative) notation? Maybe some examples? It's good to keep it in
the commit log.

There seem to be some indentation changes in the table gen, can you
refrain from committing them just now? After the change you can add as
"cosmetic changes", but right now it'll give the wrong impression.

You have added a Requires<[IsARM, HasV6T2]> to the ARM version of the
instruction, would be good to do the same with the Thumb version.

Also, this is personal, but BFI_Alt doesn't seem a good name. This is
not an alternative encoding, this is the documented in ARM ARM, so I
guess it could either get a real name (like 4-param-BFI) or the other
get its name changed.

Finally, what is the isAsmParserOnly flag? Does it mean that that
instruction can only be parsed by a disassembler? Would be good to have
it accessible from the codegen, especially because it's not a
pseudo-instruction. But that's up to the back-end people to decide.

My tuppence,

best,
--renato

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.