[PATCH] D23506: [RFC] Generate long nop instructions depending on function-specific subtarget info (version 2)

[James Y Knight via llvm-commits]

jyknight added inline comments.


> MCObjectStreamer.cpp:540
>  
> +void MCObjectStreamer::EmitArch(StringRef Token) {
> +  assert(Token.find(' ') == std::string::npos &&

Why is this in generic code. Is .arch a generic directive? This syntax is x86-specific.

> MCStreamer.cpp:33
>  
> +// Emitting .arch directive hurts compatibility with assemblers other than LLVM,
> +// so don't preserve alignment instructions across assembly by default.

I think this should be done so that it's compatible with gas. Is there some reason it can't be?

> X86AsmParser.cpp:3049
> +
> +    if (!NewSTI.isFeatureStringValid(Feature)) {
> +      Error(L, "unrecognized feature \'" + Feature + "\' in .arch directive");

These features are LLVM features (right?), not the feature strings that are used with .arch. So this isn't really okay as is, unless the llvm features happened to align with the .arch features. Which they don't.

(.arch syntax in gas: https://sourceware.org/binutils/docs/as/i386_002dArch.html#i386_002dArch)

https://reviews.llvm.org/D23506