[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

leonardchan added a comment. In D74619#1936336 , @pratlucas wrote: > Hi @leonardchan , > > I've double-checked the Neon intrinsics reference and it indeed confirms that > the only allowed value for the lane argument for `vdupq_lane_f64` is `0`: > >

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

pratlucas added a comment. Hi @leonardchan , I've double-checked the Neon intrinsics reference and it indeed confirms that the only allowed value for the lane argument for `vdupq_lane_f64` is `0`: Argument Preparation vec → Vn.1D 0 << lane << 0

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

leonardchan added a comment. Actually this may not need a reproducer since the error seems straightforward with just calling a builtin function. The main issue is that before this patch, something like #include float64x2_t func(int8x8_t x) { return __builtin_neon_splatq_lane_v(x, 1,

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

leonardchan added a comment. Hi, a bisect seems to show https://reviews.llvm.org/rGf56550cf7f12b581a237b48a7f4d8b6682d45a09 is causing us to see the following error: error: argument value 1 is outside the valid range [0, 0] v2 = vdupq_lane_f64(vget_high_f64(a.v), 1); ^

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

ostannard accepted this revision. ostannard added a comment. This revision is now accepted and ready to land. I agree with @dnsampaio here, it's better to match the existing style, and avoid irrelevant churn in the git history. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

dnsampaio added a comment. Hi, thanks for looking into this. The patch LGTM, but regarding the indentation, I don't know what would be the best practice here. We tend to like to preserve the line-git-history, but if we start ignoring the formater check, then it has no sense in they being

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

pratlucas added a comment. The clang-format pre-merge check keeps wanting me to update the indentation for the entire `ARMSIMDIntrinsicMap` and `AArch64SIMDIntrinsicMap` maps due to the change in their first entries. I believe, though, that this change would not only be out of the scope of this

[PATCH] D74619: [ARM] Enabling range checks on Neon intrinsics' lane arguments

dnsampaio added inline comments. Comment at: clang/test/CodeGen/arm-neon-range-checks.c:7 +void test_vdot_lane(int32x2_t r, int8x8_t a, int8x8_t b) { + vdot_lane_s32(r, a, b, -1); // expected-error {{argument value -1 is outside the valid range}} + vdot_lane_s32(r, a, b, 2);