david-arm added a comment. In D110258#3055488 <https://reviews.llvm.org/D110258#3055488>, @dmgreen wrote:
> If D111551 <https://reviews.llvm.org/D111551> was folded into this patch, > would it be possible to add tests for -tune-cpu enabling/disabling features > at the correct times? Similar to the comment I left on D110259 <https://reviews.llvm.org/D110259>, I don't want D111551 <https://reviews.llvm.org/D111551> to hold up the cost model changes, which are critical. I'd prefer them to be independent. I expect D111551 <https://reviews.llvm.org/D111551> to take longer to get approval, and even once approved/merged if for any reason D111551 <https://reviews.llvm.org/D111551> causes issues after merging we only have to revert that one change to AArch64.td. ================ Comment at: clang/test/Driver/aarch64-mtune.c:3 + +// There shouldn't be a default -mtune. +// RUN: %clang -target aarch64-unknown-unknown -c -### %s 2>&1 \ ---------------- dmgreen wrote: > Why do we not want to add a default tune-cpu? This was in response to an earlier review comment by @paulwalker-arm asking what benefit "-mtune=generic" provded and about restricting the patch to only add `tune-cpu` when the user has explicitly specified one. ================ Comment at: clang/test/Driver/aarch64-mtune.c:34 + +// RUN: %clang -target aarch64-unknown-unknown -c -### %s -mcpu=thunderx 2>&1 \ +// RUN: | FileCheck %s -check-prefix=mcputhunderx ---------------- dmgreen wrote: > My understanding is that -mcpu=cpu is the same as -march=something + > -mtune=cpu. Why would this case not add a -tune-cpu too? Is it because that > gets handled in llvm? I thought this was pretty standard behaviour? We're already adding -target-cpu, which implies the arch + tuning, so isn't adding -tune-cpu redundant? Not sure what value "-target-cpu=thunderx -tune-cpu=thunderx" adds really. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110258/new/ https://reviews.llvm.org/D110258 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
