transform: fix bug in StringToBytes optimization pass

Previously, this pass would convert any read-only use of a
runtime.stringToBytes call to use the original string buffer instead.
This is incorrect: if there are any writes to the resulting buffer, none
of the slice buffer pointers can be converted to use the original
read-only string buffer.

This commit fixes that bug and adds a test to prove the new (correct)
behavior.

transform/rtcalls.go

@@ -28,32 +28,44 @@ func OptimizeStringToBytes(mod llvm.Module) {
 
 		// strptr is always constant because strings are always constant.
 
-		convertedAllUses := true
+		var pointerUses []llvm.Value
+		canConvertPointer := true
 		for _, use := range getUses(call) {
 			if use.IsAExtractValueInst().IsNil() {
 				// Expected an extractvalue, but this is something else.
-				convertedAllUses = false
+				canConvertPointer = false
 				continue
 			}
 			switch use.Type().TypeKind() {
 			case llvm.IntegerTypeKind:
 				// A length (len or cap). Propagate the length value.
+				// This can always be done because the byte slice is always the
+				// same length as the original string.
 				use.ReplaceAllUsesWith(strlen)
 				use.EraseFromParentAsInstruction()
 			case llvm.PointerTypeKind:
 				// The string pointer itself.
 				if !isReadOnly(use) {
-					convertedAllUses = false
+					// There is a store to the byte slice. This means that none
+					// of the pointer uses can't be propagated.
+					canConvertPointer = false
 					continue
 				}
-				use.ReplaceAllUsesWith(strptr)
-				use.EraseFromParentAsInstruction()
+				// It may be that the pointer value can be propagated, if all of
+				// the pointer uses are readonly.
+				pointerUses = append(pointerUses, use)
 			default:
 				// should not happen
 				panic("unknown return type of runtime.stringToBytes: " + use.Type().String())
 			}
 		}
-		if convertedAllUses {
+		if canConvertPointer {
+			// All pointer uses are readonly, so they can be converted.
+			for _, use := range pointerUses {
+				use.ReplaceAllUsesWith(strptr)
+				use.EraseFromParentAsInstruction()
+			}
+
 			// Call to runtime.stringToBytes can be eliminated: both the input
 			// and the output is constant.
 			call.EraseFromParentAsInstruction()

transform/testdata/stringtobytes.ll

@@ -30,3 +30,19 @@ entry:
   call fastcc void @writeToSlice(ptr %1, i64 %2, i64 %3)
   ret void
 }
+
+; Test that pointer values are never propagated if there is even a single write
+; to the pointer value (but len/cap values still can be).
+define void @testReadSome() {
+entry:
+  %s = call fastcc { ptr, i64, i64 } @runtime.stringToBytes(ptr @str, i64 6)
+  %s.ptr = extractvalue { ptr, i64, i64 } %s, 0
+  %s.len = extractvalue { ptr, i64, i64 } %s, 1
+  %s.cap = extractvalue { ptr, i64, i64 } %s, 2
+  call fastcc void @writeToSlice(ptr %s.ptr, i64 %s.len, i64 %s.cap)
+  %s.ptr2 = extractvalue { ptr, i64, i64 } %s, 0
+  %s.len2 = extractvalue { ptr, i64, i64 } %s, 1
+  %s.cap2 = extractvalue { ptr, i64, i64 } %s, 2
+  call fastcc void @printSlice(ptr %s.ptr2, i64 %s.len2, i64 %s.cap2)
+  ret void
+}

transform/testdata/stringtobytes.out.ll

@@ -22,3 +22,13 @@ entry:
   call fastcc void @writeToSlice(ptr %1, i64 6, i64 6)
   ret void
 }
+
+define void @testReadSome() {
+entry:
+  %s = call fastcc { ptr, i64, i64 } @runtime.stringToBytes(ptr @str, i64 6)
+  %s.ptr = extractvalue { ptr, i64, i64 } %s, 0
+  call fastcc void @writeToSlice(ptr %s.ptr, i64 6, i64 6)
+  %s.ptr2 = extractvalue { ptr, i64, i64 } %s, 0
+  call fastcc void @printSlice(ptr %s.ptr2, i64 6, i64 6)
+  ret void
+}