Skip to content

Integrated-as fails to assemble these (presumably legitimate) instructions #19295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
llvmbot opened this issue Feb 21, 2014 · 10 comments
Closed
Labels
backend:ARM bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Member

llvmbot commented Feb 21, 2014

Bugzilla Link 18921
Resolution FIXED
Resolved on Dec 09, 2015 05:08
Version trunk
OS Linux
Blocks #19300
Attachments instructions emitted by gcc
Reporter LLVM Bugzilla Contributor
CC @compnerd,@rengolin,@kaomoneus

Extended Description

I've swept together a bunch of instructions which GCC emitted but which Clang's integrated-as rejects (GNU assembler does not). Most of them it says need ARM mode, but there are a couple of invalid operands which I assume are "oversized" immediates.

This is in Thumb mode. Perhaps these are being filtered against the original Thumb mode, rather than Thumb2?

I suspect that gcc could be performing some instruction substitutions which aren't officially part of the language.

I haven't pared the list down because I'm afraid I might remove something important, like a specific register number.

@llvmbot
Copy link
Member Author

llvmbot commented Feb 21, 2014

By the way... if it is the case that the list is being trivially mis-filtered, does this imply that the code LLVM generates isn't utilising the instruction set fully?

@rengolin
Copy link
Member

Even though compilers should be able to emit the whole instruction set, they are not obliged to do so on user code, and not many of them do. A good example is NEON, that can be generated either via vectorization passes or intrinsics, with the former never using the whole set and the latter rarely using the more obscure ones.

ARM has used the MC-Hammer to make sure the encodings are correct to/from the LLVM assembler, so if there are some encodings missing, I'd suggest you run them over MC-Hammer first. If these are simply missing, implementing support for them should be trivial. If not, we'll have trouble accepting bad encodings "just because some other compiler emits them".

@kaomoneus
Copy link
Contributor

Everything fixed now, except "vmov.i32 dn, 0xffffffff"
According to ARM reference this instruction is incorrect, since there is no way to encode 0xffffffff immediate, see "A7.4.6 One register and a modified immediate value".

Though, GCC converts this instruction into "vmov.i8 dn, #​255". Should we do the same? What is the best way to do it? InstAlias? PseudoExpand?
Actually it could be replaced with "vmov.i64 dn, 0xffffffff" as well.

@compnerd
Copy link
Member

compnerd commented Aug 3, 2014

I can understand the desire to be compatible GAS, however, I think that emitting a diagnostic here that the value is not representable is better. If we were to adopt the behavior that has been implemented by GAS, we should still emit a diagnostic indicating the silent truncation. Simply fixing the assembly on the input seems like a better choice.

@kaomoneus
Copy link
Contributor

Hi Saleem,
Sorry I forgot to notify bug subscribers about related phabricator ticket:
http://reviews.llvm.org/D3315
We have fixed it in r207080.
Yeah, got your idea. For now, we just follow GAS rules silently. I totally agree that also it would be good to emit the warning at least.

@rengolin
Copy link
Member

rengolin commented Aug 4, 2014

Hi Slaeem,

This is not GAS compatibility, but GCC compatibility. One of the IAS design goals was to be able to (within reason) be able to consume any code generated by GCC as well as obviously LLVM.

This requirement comes from the fact that a multi-stage compilation that intermixes Clang and GCC would fail if we didn't. Intermixing Clang and GCC will be a lot more common in the future with people writing wrappers to run Clang first and, on failure, GCC. Even though I don't like the idea, I've heard about it quite a few times and people will do it.

Though, I completely agree with the warning, I think we ought to do that to every non-standard (in this case UAL) syntax.

cheers,
--renato

@llvmbot
Copy link
Member Author

llvmbot commented Aug 4, 2014

I've often found it a nuisance trying to remember which immediates go with which data type when using vmov-immediate. I know that various repeating patterns of 1 and 0 bits can be written into a NEON register, but I don't always remember when those patterns are a .i8 operand and when they're a .i16, .i32, or .i64 operand.

Really the mapping between bit pattern and lane size is arbitrary, and not something worth memorising if the assembler could simply work it out. I'd rather just say what I mean and have the assembler work it out. Although the assembler must at least do what I said when I said something legal.

@rengolin
Copy link
Member

rengolin commented Aug 4, 2014

Simon, in your case, a warning with a fixme would do just fine. You'd never have to remember all variations in your life, and just trust the assembler to tell you which ones to use.

Silently ignoring "invalid" syntax is not a good way, IMO. Even though the syntax is more convoluted than it should, and the assembler could potentially work it out what you really "meant".

Compilers are often wrong about the true meaning of things to be on the safe side and that's not wrong.

If you're going down the path of writing assembly, you should expect to know a few gory details...

@rengolin
Copy link
Member

rengolin commented Dec 9, 2015

This seems to have been fixed a long while ago.

@rengolin
Copy link
Member

mentioned in issue #19300

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:ARM bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

4 participants