First of all, please I would appreciate that you try to not confuse my limited use of English with stupidity or lack or criteria in other subjects. I’m not English native, so please keep that in mind. You have been significantly helpful in the recent past so please keep on.
Interestingly, you made a mention of a related but not identical issue. It is true that most (or all) processors support constant shift instructions so that LLVM can safely assume that constant shifts are legal. I don’t think that LLVM makes any explicit assumption that variable shifts are legal too though, but if it does, then this is not either a problem.
We need to make the distinction between constant shifts and variable shifts. The later are generally supported only by the major processors, but not in the general case. In particular the AVR and the MSP430 targets do not have native support for them. In both cases variable shifts are dealt with by means of a custom inserter. This is just a possible way to implement them, but not the easiest or the cleaner one in my opinion.
Another possibility is to implement Custom lower for constant shifts, but to generate a lib call (or an intrinsic) for the variable shifts. The lib call can be generated as part of the Custom lowering, and this is the implementation that I chose for my target, however, the LLVM infrastructure does not make this particularly straightforward for two reasons:
- First, there’s no working support for ‘LibCall’ lowering of shifts, this is broken by the omission of the case statement that I mentioned,
- Second, there’s no simple way that I am aware to make LLVM to create a lib call from the Custom lowering implementation;
I am not going to strongly suggest how to solve this problem, but just to state that there’s an actual problem or at least an oddity on the way that shifts are (or can be) dealt with.
To me, the first thing we need to do is to actually have LibCall operationActions working for shifts. This is also as a matter of code consistency if you want, and doing this alone doesn’t break anything: ultimately it’s the responsibility of the target to chose whether it uses it or not.
Additionally, we should specify a way to fall through to LibCall lowering from a Custom lower implementation, if required or requested by the target. For example by providing a public path or a hook to ConvertNodeToLibCall, and/or by making the Custom implementation returning something that is recognised by the SelectionDAGLegalize code as a request for a LibCall fall through. Currently, the fall through mechanism after Custom lowering goes directly to the Expand code
Yet another approach would be that LLVM would explicitly made a distinction between constant and variable shifts, so that they can be lowered in separated ways, but I rather prefer the approach above because it’s more generic and it would widen the custom lowering implementation possibilities for targets with partial support of LLVM instructions.
In my opinion, it’s not right to consider any "out of tree targets", (whatever that means) as not worth for LLVM improvement. The LLVM project has traditionally focused on major processors for a good reason, but the grow of the IoT and the recent popularisation of many 8 and 16 bit architectures for embedded and control applications make it worth a LLVM focus shift to the particular needs of these smaller processors. Incidentally, the AVR and the MSP430 still remain as ‘experimental’ targets.
Just my two cents anyway, and sorry for my English.