[RFC] [X86] Emit unaligned vector moves on avx machine with option control.

Hi all.

We want to make a patch to always emit unaligned vector move instructions on AVX machine with option control. We do this for the following reason:

This is not a principled change – it avoids a problem arising from one use of alignment information, but there are other uses of alignment in LLVM, and those will still cause problems, potentially less clearly. So, I think that this will not be a useful option to provide to users, in this form.

What I suspect you actually want here is an option to tell Clang not to infer load/store alignments based on object types or alignment attributes – instead treating everything as being potentially aligned to 1 unless the allocation is seen (e.g. global/local variables). Clang would still need to use the usual alignment computation for variable definitions and structure layout, but not memory operations. If clang emits “load … align 1” instructions in LLVM IR, the right thing would then happen in the X86 backend automatically.

My initial inclination is that this feature is also not particularly worthwhile to implement, but I’m open to being convinced that this is indeed valuable enough to be worthwhile. It should actually work reliably, and is somewhat in line with other such “not-quite-C” flags we provide (e.g. -fno-delete-null-pointer-checks). Of course, even with such an implementation, you can still have a problem with user code depending on alignof() returning a reliable answer (e.g., llvm::PointerUnion). Not much can be done about that.

When would you recommend a user should use this flag as proposed? Anytime they moved code from icc? Or after they encounter an exception, should they use this flag to get rid of the exception rather than using tools like ubsan to find the bug in their code? Where would we document recommendations so that users know the tradeoffs and risks?

Your patch is only doing this for AVX and not SSE because folding loads requires alignment with SSE, but not AVX. So users that need to support non-AVX CPUs have to fix their bugs and can’t sweep them away with this flag.

I’m somewhat ok with unconditionally using unaligned instructions on AVX because then there is no command line option to explain to users. But then there’s probably a group of people out there that want the alignment check.

~Craig

+1 to what James said. My reaction to the original proposal is a strong -1, and James did a good job of explaining why.

Philip

Hi, James Y Knight.

I’m not sure if you misunderstood this patch. This patch won’t change any alignment information in IR and MI, which means ‘load…align 32’ will always keep the alignment information but select ‘vmovups’ instead of ‘vmovaps’ during ISEL. It can be simply considered that the only thing this patch does is to replace the aligned-move mnemonic with the unaligned-move mnemonic (in fact, we shouldn’t call it replace but emit unaligned). I think there is no impact on optimization or code layout.

After discussion, we think this option more like changing the behavior when process with unaligned memory: raising exception or accepting performance degradation. Maybe the option is more like “no-exception-on-unalginedmem”. We do have some users want this feature. They can accept “run slow” but do not want exception.

Thanks.

Chen Liu.

Hi, James Y Knight.

I’m not sure if you misunderstood this patch. This patch won’t change any alignment information in IR and MI, which means ‘load…align 32’ will always keep the alignment information but select ‘vmovups’ instead of ‘vmovaps’ during ISEL. It can be simply considered that the only thing this patch does is to replace the aligned-move mnemonic with the unaligned-move mnemonic (in fact, we shouldn’t call it replace but emit unaligned). I think there is no impact on optimization or code layout.

Yes – I understood that, and that is exactly why this patch is not OK. Giving LLVM incorrect information about the alignment of objects causes problems other than just the emission of movaps instructions – that alignment information is correct gets relied upon throughout the optimization pipeline.

So, a command-line option to “fix” only that one instruction is not something which we can reasonably provide, because it will not reliably fix users’ problems. A program which is being “mis”-compiled due to the use of misaligned objects might still be miscompiled by LLVM when using your proposed patch. (“mis” in quotes, since the compiler is correctly compiling the code according to the standard, even if not according to the user’s expectations).

The second paragraph of my original email describes an alternative patch that you could write, which would reliably fix such miscompilation – effectively creating a variant of C where creating and accessing misaligned objects has fully defined behavior. (And, just to reiterate, my initial feeling is that creating such an option is not a worthwhile endeavor, but I could be persuaded otherwise.)

Yes, replacing aligned move instruction with unaligned move instruction doesn’t solve all the issue that happens in optimization pipeline, but it doesn’t make things worse. One advantage for unaligned move is that it makes the behavior the same no matter the mov instruction is folded or not. Do you think it is worth to support this feature if compiler can help users avoid changing their complex legacy code?

Thanks

Yuanke

IMO, no. We should encourage sanitizers instead.

From experience, any code base where porting trips across this probably also has a bunch of other undefined behavior which is causing less obvious miscompiles, and also need found and fixed. That’s why we have sanitizers.

Philip

I’ve debated whether to chime in, and decided it can’t hurt.

Sony had to do a similar downstream patch for PS4. Our use-case is pretty constrained, though. There’s only one toolchain, there’s only one target chip, so we don’t have any portability considerations to think about. What we do have are games shipping on DVD that can’t be re-released and can’t even necessarily be patched, and a strict backward compatibility requirement. So, if there’s a game out there that didn’t happen to follow all the alignment requirements, and it worked on console version 1.00, it still has to be working on version 100.00. (FTR, we’re currently on about 8.00.)

I don’t think we ever seriously considered upstreaming our patch. The circumstances where it’s really necessary do exist, but are pretty limited.

I don’t think arguments of the form “it’s okay because X Y and Z” are going to be persuasive. “We have this situation in the following circumstances” might help people understand.

–paulr

Paul, was your patch attached to a command line option, if so what was the default, or did you just always use unaligned instructions?

I believe strongly that we should not add an option which makes it sound like it makes unaligned access work, when we know for a fact that optimization passes make use of the alignment information and will also break such misaligned-object-using code. Worse, we also can predict that even more such optimizations will be added in future versions of llvm, and break such code more. Offering such an option which seems like it would do what they want, but which doesn’t actually, is a perfect recipe for creating unhappy users.

That’s why I’ve been saying over and over that if we do end up providing some “make unaligned access work” option, it needs to make it actually work, reliably, both now and in the future.

We conditioned it on the PS4 target; no option. So, PS4 consistently uses unaligned instructions for (temporal) vector load/store.

We get occasional downstream test failures because of this, which we deal with.

–paulr

This sounds like the -fmax-type-align flag:
https://clang.llvm.org/docs/UsersManual.html#controlling-code-generation

Explicit alignment attributes are still honored, so some aligned vector instructions may be generated. However, the documentation describes essentially this exact use case.

This sounds like the -fmax-type-align flag:

Well, no, at least not for the PS4 case. In our case, the type had an alignment attribute but the caller didn’t make sure the allocated memory was aligned properly. The -fmax-type-align flag explicitly doesn’t do anything in that case, if I’m reading it correctly. (Yes, it’s a bug. Yes, sanitizers or other testing could have found it. No, there is no opportunity to do any of the things that would have fixed it correctly.)

Really what we did was effectively this: Pretend X86 doesn’t have a VMOVAPS opcode. That’s all. Nothing about memory/operand alignment attributes was modified, IR is unchanged. Pretend that one machine opcode is missing. Can’t possibly affect anything about IR optimizations, maybe something post-ISel would be different but even that is hard to imagine. (As best I can remember, the only test updates we had to make were to change things like “vmovaps” to “vmov{{u|a}}ps” and done.) It’s like we did s/movaps/movups/g on the assembly output.

I still can’t say I think it should be appropriate to do upstream—no real info yet on Intel’s problem case–but I hope this explains why the bigger hammer (i.e., get Clang involved) doesn’t seem necessary or appropriate.

–paulr

Right, I get that this doesn’t match what you are doing for PS4, and it doesn’t match what Chen3 Liu proposed. To James’s point, the -fmax-type-align flag is more principled because it powers down all the other LLVM optimizations that assume aligned pointers have zeros in the low bits.

As for how to handle explicit alignment attributes that don’t come from type information, maybe we could revisit that behavior, or conditionalize it with a flag. I just mean to say that there is prior art for this direction. We should continue in the direction of a complete solution from the frontend, rather than adding a workaround in the backend.

Before we completely float away from adding this to LLVM, consider that MSVC behavior is already like ICC: https://godbolt.org/z/o4eaqGv9v

And GCC folks are saying they could add an option for compatibility.

The Intel’s cases where this is needed is exactly for robustness of interoperability with the already released software.

What if we didn’t use aligned instructions by default like what PS4 did. And then had a command line option that would “enable alignment exceptions” if someone wants them. Maybe that option should also disable memory folding since memory folding never checks alignment with AVX? Do other targets that have vectors have alignment exceptions like this? We’re not obligated to emit code that detects alignment errors. And we already don’t if the load gets folded. It seems the problem with the current proposal is that once you have the exception, setting a flag to make it go away is the wrong response.

Reid, I’m not clear why anyone would want to “power down” the alignment-aware optimizations? How does that benefit anyone? For example…

Let’s postulate a target that has only non-trapping load/store instructions; maybe they go faster on aligned addresses, but don’t trap on unaligned addresses. It has been a few decades but I think VAX worked this way.

Would you insist we should power-down the alignment-aware optimizations for this target? Just because the hardware couldn’t require aligned data? I hope not.

The conclusion must be, then, that there is no relationship between the existence of trapping/non-trapping instruction behavior for a given target, and how the frontend and middle-end should behave.

Therefore, we can’t insist on the front-end slapping “align 1” on everything just because the target doesn’t trap a non-aligned load.

Therefore, the choice of trapping/non-trapping instruction behavior in the X86 target specifically, has no necessary relationship to how alignment is thought of in the front-end/middle-end.

HTH,

–paulr

Reid, I’m not clear why anyone would want to “power down” the alignment-aware optimizations? How does that benefit anyone? For example…

Let’s postulate a target that has only non-trapping load/store instructions; maybe they go faster on aligned addresses, but don’t trap on unaligned addresses. It has been a few decades but I think VAX worked this way.

Would you insist we should power-down the alignment-aware optimizations for this target? Just because the hardware couldn’t require aligned data? I hope not.

The conclusion must be, then, that there is no relationship between the existence of trapping/non-trapping instruction behavior for a given target, and how the frontend and middle-end should behave.

Therefore, we can’t insist on the front-end slapping “align 1” on everything just because the target doesn’t trap a non-aligned load.

Certainly, it’s entirely valid for a target to not trap on an unaligned load. We have many such targets. A target trapping on misaligned loads isn’t a required feature. (If users want to reliably diagnose misalignment bugs, -fsanitize=alignment is the way to do so.)

Therefore, the choice of trapping/non-trapping instruction behavior in the X86 target specifically, has no necessary relationship to how alignment is thought of in the front-end/middle-end.

If the proposal here had been: "We should switch X86 from using movaps (alignment-checking) to movups (non-alignment-checking), because movups has a smaller encoding size (or is faster to execute on new microarchitectures, or …), there’d be no problem.

But, that is not what’s being proposed here. This proposal is to switch to movups as a workaround for software that has undefined behavior due to misaligned objects. That is misguided, because the proposed change does not fix such code! That the movaps instruction traps in such programs is like a proverbial “canary in a coal mine”. It’s a result of your program containing alignment-related UB. Removing the canary prevents you from having a dead canary, but it doesn’t prevent the mine from exploding.

I have the feeling folks aren’t understanding what exactly I’m talking about w.r.t. alignment-related breakage. There’s at least three things LLVM can do with alignment information today.

  1. Most obviously, it allows generation of hardware load instructions that require a certain alignment (MOVAPS on X86, LDM on ARM, etc.).
  2. It enables known-bits analysis on pointers: “ptr & 0x3” is optimized to 0 if ptr is known to have alignment >= 4. Example: int foo(int& x) { return ((uintptr_t)&x) & 0x3; }
  3. It can assist with alias analysis: if both addr1 and addr2 have align 8, then a 4-byte load from (addr1 + 0) cannot possibly alias a 4-byte load from (addr2 + 4). This is true even without TBAA, and even if know nothing else about the relationship between addr1 and addr2. (I don’t have an example of this – it looks like llvm may not be doing as good a job here as it could, but I definitely recall reading code which purported to implement this.)

The initial proposal only addresses the first issue, leaving users who depend on this are in an extremely precarious position – liable to be broken by any future optimization improvement.

Wow, thanks! Somehow I’ve missed that this flag has existed all this time. ISTM that it would be reasonable to modify -fmax-type-align to override even an explicit alignment attribute on the type (or typedef).

It looks like -fmax-type-align is barely used in the wild, except that -fmax-type-align=16 is default for Darwin platforms (since commit bcd82afad64a22b15000de350d075b10f2de273a). It’s unclear to me what purpose that default is really serving, however, given that the only types with greater “native” alignment than 16 are vector types, and typically used vector typedefs already have an alignment specified, such as typedef float __m256 __attribute__ ((__vector_size__ (32), __aligned__(32)));. So the most-commonly-used vector types are exempted from the effect of the flag, anyways…