arsenm added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:2030-2031 + bool EnabledForTarget = TEntry->second; + if (EnabledForTarget != EnabledForFunc) + return; + } ---------------- jmmartinez wrote: > arsenm wrote: > > Early return breaks the other features > I did not understand this remark. > > If the features are not compatible, we do not add a "target-features" entry > in the new "FuncAttrs". Then, the old "target-features" entry is kept in the > Function coming from the builtin. > > If you think it would be better to set the target-features in FuncAttrs to > the old value in any case. If that's the case I've added the following code: > > if (EnabledForTarget != EnabledForFunc) { > FuncAttr.addAttribute(FFeatures); > return; > } You find an incompatible feature and then discontinue processing any further features by early exiting. I expect this to act like an append to any features already present. The incompatibility is at an individual feature level, not the group ================ Comment at: clang/lib/CodeGen/CGCall.cpp:2034 + } + + FuncAttr.addAttribute("target-features", llvm::join(MergedFeatures, ",")); ---------------- jmmartinez wrote: > arsenm wrote: > > Really it would be less bad if the incompatible functions were not imported > > rather than the backend pass > I thought it was possible to have functions with incompatible features in the > same module. > e.g. one function compiled with some instruction set support, one without, > and an ifunc that resolves to one or the other. > > Maybe it's not the case in the context of `-mlink-builtin-bitcode`? The truth is this system isn't really well considered. We don't have real ifunc support and we probably shouldn't be using subtargets for cases with incompatible encodings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159206/new/ https://reviews.llvm.org/D159206 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits