On Mon, Apr 08, 2024 at 12:33:39PM +0000, Jiang, Haochen wrote: > Sorry for the late response since I am on vacation for now. > > > As the following testcase shows, the above change was incorrect. > > > > Using aes isa for the second alternative is obviously wrong, aes is enabled > > whenever -maes is, regardless of -mavx or -mno-avx, so the above change > > means that for -maes -mno-avx RA can choose, either it matches the first > > alternative with the dup operand, or it matches the second one (but that > > is of course wrong because vaesenc VEX encoded insn needs AES & AVX CPUID). > > When I wrote that patch, I suppose it will never match the second one when > AVX is not enabled because it will immediately drop to the first one so the > second one is automatically AES && AVX, which is tricky here.
Before the -mvaes changes the alternatives were noavx,avx isa and so clearly it was either the first alternative is the solely available, or the second, depending on TARGET_AVX. But with noavx,aes on the first alternative is enabled only for !TARGET_AVX, but the second one whenever TARGET_AES, which is both if !TARGET_AVX and TARGET_AVX. So, the RA is free to consider both alternatives, and because the first one is more restrictive (requires output matching input), if there is a match between those, it will use the first alternative, but if there isn't, it will happily use the second alternative. > LLVM always had less restrictions on ISA under such circumstances, I would > like to > stick to how SDM did when implementing that, which is a little conservative. > > However, I am also ok with VAES implying AES if there is no real HW that has > VAES w/o AES to reduce complexity in this scenario. I'm fine with -mvaes not implying -maes, just want to mention that it is fairly user visible thing and so we shouldn't be changing it after deciding if we do it one way or another. Now, I thought -mvaes was added in GCC 14, but it has been around for a few years, so that means it is likely a bad idea to change it now. Jakub