llc: Change behavior of -mattr with existing attribute

[arsenm]

Summary

Append this to the existing target-features attribute on the function.

Some flags ignore existing attributes, and some overwrite them. Move
towards consistently respecting existing attributes if present. Since
target features act as a state machine on their own, append to the
function attribute. The backend default added feature list, function
attributes, and -mattr will all be appended together, and the later
features can individually toggle the earlier settings.

[arsenm]

ping

[arsenm]

ping

[craig.topper]

LGTM

[arsenm]

67ec8744d7e72b50a5db5038c9643584ce57cb0c

llvm/include/llvm/CodeGen/CommandFlags.inc

 LLVM_ATTRIBUTE_UNUSED static void
 setFunctionAttributes(StringRef CPU, StringRef Features, Function &F) {
   auto &Ctx = F.getContext();
   AttributeList Attrs = F.getAttributes();
   AttrBuilder NewAttrs;
 
   if (!CPU.empty() && !F.hasFnAttribute("target-cpu"))
     NewAttrs.addAttribute("target-cpu", CPU);
-  if (!Features.empty())
+  if (!Features.empty()) {
+    // Append the command line features to any that are already on the function.
+    StringRef OldFeatures
+      = F.getFnAttribute("target-features").getValueAsString();
+    if (OldFeatures.empty())
       NewAttrs.addAttribute("target-features", Features);
+    else {
+      SmallString<256> Appended(OldFeatures);
+      Appended.push_back(',');
+      Appended.append(Features);
+      NewAttrs.addAttribute("target-features", Appended);
+    }
+  }
   if (FramePointerUsage.getNumOccurrences() > 0) {
     if (FramePointerUsage == llvm::FramePointer::All)
       NewAttrs.addAttribute("frame-pointer", "all");
     else if (FramePointerUsage == llvm::FramePointer::NonLeaf)
       NewAttrs.addAttribute("frame-pointer", "non-leaf");
     else if (FramePointerUsage == llvm::FramePointer::None)
       NewAttrs.addAttribute("frame-pointer", "none");
   }

llvm/test/CodeGen/WebAssembly/target-features.ll

 ; RUN: llc < %s | FileCheck %s --check-prefixes CHECK,ATTRS
 ; RUN: llc < %s -mattr=+simd128 | FileCheck %s --check-prefixes CHECK,SIMD128
 ; RUN: llc < %s -mcpu=bleeding-edge | FileCheck %s --check-prefixes CHECK,BLEEDING-EDGE
 
 ; Test that codegen emits target features from the command line or
 ; function attributes correctly and that features are enabled for the
 ; entire module if they are enabled for any function in the module.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
-define void @foo(i32* %p1, float %f2) #0 {
+define void @fn_atomics(i32* %p1, float %f2) #0 {
   %a = atomicrmw min i32* undef, i32 42 seq_cst
   %v = fptoui float %f2 to i32
   store i32 %v, i32* %p1
   ret void
 }
 
-define void @bar(i32* %p1, float %f2) #1 {
+define void @fn_nontrapping_fptoint(i32* %p1, float %f2) #1 {
   %a = atomicrmw min i32* undef, i32 42 seq_cst
   %v = fptoui float %f2 to i32
   store i32 %v, i32* %p1
   ret void
 }
 
 attributes #0 = { "target-features"="+atomics" }
 attributes #1 = { "target-features"="+nontrapping-fptoint" }
 
-
-; CHECK-LABEL: foo:
+; CHECK-LABEL: fn_atomics:
 
 ; Expanded atomicrmw min
 ; ATTRS:       loop
-; ATTRS:       i32.atomic.rmw.cmpxchg
-; SIMD128-NOT: i32.atomic.rmw.cmpxchg
+; CHECK:       i32.atomic.rmw.cmpxchg
 ; ATTRS:       end_loop
 
 ; nontrapping fptoint
-; ATTRS:       i32.trunc_sat_f32_u
-; SIMD128-NOT: i32.trunc_sat_f32_u
+; CHECK:       i32.trunc_sat_f32_u
 ; ATTRS:       i32.store
 
-; `bar` should be the same as `foo`
-; CHECK-LABEL: bar:
+; `fn_nontrapping_fptoint` should be the same as `fn_atomics`
+; CHECK-LABEL: fn_nontrapping_fptoint:
 
 ; Expanded atomicrmw min
 ; ATTRS:       loop
-; ATTRS:       i32.atomic.rmw.cmpxchg
-; SIMD128-NOT: i32.atomic.rmw.cmpxchg
+; CHECK:       i32.atomic.rmw.cmpxchg
 ; ATTRS:       end_loop
 
 ; nontrapping fptoint
-; ATTRS:       i32.trunc_sat_f32_u
-; SIMD128-NOT: i32.trunc_sat_f32_u
+; CHECK:       i32.trunc_sat_f32_u
 ; ATTRS:       i32.store
 
 ; CHECK-LABEL: .custom_section.target_features,"",@
 
 ; +atomics, +nontrapping-fptoint
 ; ATTRS-NEXT: .int8 2
 ; ATTRS-NEXT: .int8 43
 ; ATTRS-NEXT: .int8 7
 ; ATTRS-NEXT: .ascii "atomics"
 ; ATTRS-NEXT: .int8 43
 ; ATTRS-NEXT: .int8 19
 ; ATTRS-NEXT: .ascii "nontrapping-fptoint"
 
-; -atomics, +simd128
-; SIMD128-NEXT: .int8 2
-; SIMD128-NEXT: .int8 45
+; +atomics, +simd128
+; SIMD128-NEXT: .int8 3
+; SIMD128-NEXT: .int8 43
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "atomics"
 ; SIMD128-NEXT: .int8 43
+; SIMD128-NEXT: .int8 19
+; SIMD128-NEXT: .ascii  "nontrapping-fptoint"
+; SIMD128-NEXT: .int8 43
 ; SIMD128-NEXT: .int8 7
 ; SIMD128-NEXT: .ascii "simd128"
 
 ; +atomics, +nontrapping-fptoint, +sign-ext, +simd128
 ; BLEEDING-EDGE-NEXT: .int8   5
 ; BLEEDING-EDGE-NEXT: .int8   43
 ; BLEEDING-EDGE-NEXT: .int8   7
 ; BLEEDING-EDGE-NEXT: .ascii  "atomics"

llvm/test/Other/opt-override-mcpu-mattr.ll

 ; RUN: opt < %s -mtriple=x86_64-apple-darwin -mcpu=broadwell -mattr=+avx2 -S | FileCheck %s
 
 ; Check that opt can add but not rewrite function attributes
 ; target-cpu and target-features using command line options -mcpu and
 ; -mattr.
 
-; CHECK: attributes #0 = { nounwind readnone ssp uwtable "target-cpu"="broadwell" "target-features"="+avx2" "use-soft-float"="false" }
-; CHECK: attributes #1 = { nounwind readnone ssp uwtable "target-cpu"="core2" "target-features"="+avx2" "use-soft-float"="false" }
+; CHECK: attributes #0 = { nounwind readnone ssp uwtable "target-cpu"="broadwell" "target-features"="+ssse3,+cx16,+sse,+sse2,+sse3,+avx2" "use-soft-float"="false" }
+; CHECK: attributes #1 = { nounwind readnone ssp uwtable "target-cpu"="core2" "target-features"="+ssse3,+cx16,+sse,+sse2,+sse3,+avx2" "use-soft-float"="false" }
 
 define i32 @no_target_cpu() #0 {
 entry:
   ret i32 0
 }
 
 define i32 @has_targe_cpu() #1 {
 entry:
   ret i32 0
 }
 
 attributes #0 = { nounwind readnone ssp uwtable "target-features"="+ssse3,+cx16,+sse,+sse2,+sse3" "use-soft-float"="false" }
 attributes #1 = { nounwind readnone ssp uwtable "target-cpu"="core2" "target-features"="+ssse3,+cx16,+sse,+sse2,+sse3" "use-soft-float"="false" }