Generated find_pinvoke code appears to be correct

Author: grendello

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrFunctionBody.cs

@@ -171,7 +171,7 @@ public LlvmIrFunctionBody (LlvmIrFunction func, LlvmIrFunction.FunctionState fun
 		previousLabel = implicitStartBlock = new LlvmIrFunctionImplicitStartLabel (functionState.StartingBlockNumber);
 	}
 
-	public void Add (LlvmIrFunctionLabelItem label)
+	public void Add (LlvmIrFunctionLabelItem label, string? comment = null)
 	{
 		label.WillAddToBody (this, functionState);
 		if (definedLabels.Contains (label.Name)) {
@@ -189,13 +189,16 @@ public void Add (LlvmIrFunctionLabelItem label)
 		precedingBlock1 = previousLabel;
 		previousLabel = label;
 
-		var comment = new StringBuilder (" preds = %");
-		comment.Append (precedingBlock1.Name);
-		if (precedingBlock2 != null) {
-			comment.Append (", %");
-			comment.Append (precedingBlock2.Name);
+		if (comment == null) {
+			var sb = new StringBuilder (" preds = %");
+			sb.Append (precedingBlock1.Name);
+			if (precedingBlock2 != null) {
+				sb.Append (", %");
+				sb.Append (precedingBlock2.Name);
+			}
+			comment = sb.ToString ();
 		}
-		label.Comment = comment.ToString ();
+		label.Comment = comment;
 	}
 
 	public void Add (LlvmIrFunctionBodyItem item)

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrInstructions.cs

@@ -73,6 +73,8 @@ protected void WriteValue (GeneratorWriteContext context, Type type, object? val
 			context.Output.Write ("null");
 		} else if (value is LlvmIrVariable variable) {
 			context.Output.Write (variable.Reference);
+		} else if (value is bool) {
+			context.Output.Write ((bool)value ? "true" : "false");
 		} else {
 			context.Output.Write (MonoAndroidHelper.CultureInvariantToString (value));
 		}
@@ -510,26 +512,48 @@ protected override void WriteBody (GeneratorWriteContext context)
 
 	public class Phi : LlvmIrInstruction
 	{
+		sealed class Node
+		{
+			public readonly LlvmIrVariableReference? Variable;
+			public readonly LlvmIrFunctionLabelItem? Label;
+
+			public Node (LlvmIrVariableReference? variable, LlvmIrFunctionLabelItem? label)
+			{
+				Variable = variable;
+				Label = label;
+			}
+		}
+
 		LlvmIrVariable result;
-		LlvmIrVariable val1;
-		LlvmIrFunctionLabelItem label1;
-		LlvmIrVariable val2;
-		LlvmIrFunctionLabelItem label2;
+		readonly List<Node> nodes;
+
+		public Phi (LlvmIrVariable result)
+			: base ("phi")
+		{
+			nodes = new ();
+			this.result = result;
+		}
 
 		/// <summary>
 		/// Represents the `phi` instruction form we use the most throughout marshal methods generator - one which refers to an if/else block and where
 		/// **both** value:label pairs are **required**.  Parameters <paramref name="label1"/> and <paramref name="label2"/> are nullable because, in theory,
 		/// it is possible that <see cref="LlvmIrFunctionBody"/> hasn't had the required blocks defined prior to adding the `phi` instruction and, thus,
 		/// we must check for the possibility here.
 		/// </summary>
-		public Phi (LlvmIrVariable result, LlvmIrVariable val1, LlvmIrFunctionLabelItem? label1, LlvmIrVariable val2, LlvmIrFunctionLabelItem? label2)
+		public Phi (LlvmIrVariable result, LlvmIrVariableReference val1, LlvmIrFunctionLabelItem? label1, LlvmIrVariableReference val2, LlvmIrFunctionLabelItem? label2)
 			: base ("phi")
 		{
 			this.result = result;
-			this.val1 = val1;
-			this.label1 = label1 ?? throw new ArgumentNullException (nameof (label1));
-			this.val2 = val2;
-			this.label2 = label2 ?? throw new ArgumentNullException (nameof (label2));
+
+			nodes = new () {
+				new Node (val1, label1 ?? throw new ArgumentNullException (nameof (label1))),
+				new Node (val2, label2 ?? throw new ArgumentNullException (nameof (label2))),
+			};
+		}
+
+		public void AddNode (LlvmIrFunctionLabelItem label, LlvmIrVariableReference? variable)
+		{
+			nodes.Add (new Node (variable, label));
 		}
 
 		protected override void WriteValueAssignment (GeneratorWriteContext context)
@@ -541,14 +565,42 @@ protected override void WriteValueAssignment (GeneratorWriteContext context)
 		protected override void WriteBody (GeneratorWriteContext context)
 		{
 			context.Output.Write (LlvmIrGenerator.MapToIRType (result.Type, context.TypeCache));
+			context.IncreaseIndent ();
+
+			bool first = true;
+			foreach (Node node in nodes) {
+				if (!first) {
+					context.Output.WriteLine (',');
+				} else {
+					first = false;
+					context.Output.WriteLine ();
+				}
+				context.Output.Write (context.CurrentIndent);
+				WriteNode (context, node);
+			}
+			context.DecreaseIndent ();
+
+			// context.Output.Write (" [");
+			// context.Output.Write (val1.Reference);
+			// context.Output.Write (", %");
+			// context.Output.Write (label1.Name);
+			// context.Output.Write ("], [");
+			// context.Output.Write (val2.Reference);
+			// context.Output.Write (", %");
+			// context.Output.Write (label2.Name);
+			// context.Output.Write (']');
+		}
+
+		void WriteNode (GeneratorWriteContext context, Node node)
+		{
+			if (node.Label == null) {
+				throw new NotImplementedException ("Internal error: null labels not implemented");
+			}
+
 			context.Output.Write (" [");
-			context.Output.Write (val1.Reference);
-			context.Output.Write (", %");
-			context.Output.Write (label1.Name);
-			context.Output.Write ("], [");
-			context.Output.Write (val2.Reference);
+			context.Output.Write (node.Variable == null ? "null" : node.Variable.Reference);
 			context.Output.Write (", %");
-			context.Output.Write (label2.Name);
+			context.Output.Write (node.Label.Name);
 			context.Output.Write (']');
 		}
 	}
@@ -594,6 +646,13 @@ public Store (LlvmIrVariable from, LlvmIrVariable to)
 			this.to = to;
 		}
 
+		public Store (object from, LlvmIrVariable to)
+			: base (Opcode)
+		{
+			this.from = from;
+			this.to = to;
+		}
+
 		/// <summary>
 		/// Stores `null` in the indicated variable
 		/// </summary>
@@ -605,7 +664,8 @@ public Store (LlvmIrVariable to)
 
 		protected override void WriteBody (GeneratorWriteContext context)
 		{
-			string irType = LlvmIrGenerator.MapToIRType (to.Type, context.TypeCache, out ulong size, out bool isPointer);
+			Type type = from?.GetType () ?? to.Type;
+			string irType = LlvmIrGenerator.MapToIRType (type, context.TypeCache, out ulong size, out bool isPointer);
 			context.Output.Write (irType);
 			context.Output.Write (' ');
 
@@ -687,6 +747,7 @@ protected override void WriteBody (GeneratorWriteContext context)
 				context.Output.Write (", label %");
 				context.Output.Write (label.Name);
 				if (!String.IsNullOrEmpty (comment)) {
+					context.Output.Write (' ');
 					context.Generator.WriteCommentLine (context, comment);
 				} else {
 					context.Output.WriteLine ();

src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrVariable.cs

@@ -21,12 +21,45 @@ enum LlvmIrVariableNumberFormat
 	Decimal,
 }
 
-abstract class LlvmIrVariable : IEquatable<LlvmIrVariable>
+abstract class LlvmIrVariableReference
 {
 	public abstract bool Global                    { get; }
 	public abstract string NamePrefix              { get; }
-
 	public string? Name                            { get; protected set; }
+
+	/// <summary>
+	/// Returns a string which constitutes a reference to a local (using the <c>%</c> prefix character) or a global
+	/// (using the <c>@</c> prefix character) variable, ready for use in the generated code wherever variables are
+	/// referenced.
+	/// </summary>
+	public virtual string Reference {
+		get {
+			if (String.IsNullOrEmpty (Name)) {
+				throw new InvalidOperationException ("Variable doesn't have a name, it cannot be referenced");
+			}
+
+			return $"{NamePrefix}{Name}";
+		}
+	}
+
+	protected LlvmIrVariableReference (string name)
+	{
+		Name = name;
+	}
+}
+
+class LlvmIrGlobalVariableReference : LlvmIrVariableReference
+{
+	public override bool Global => true;
+	public override string NamePrefix => "@";
+
+	public LlvmIrGlobalVariableReference (string name)
+		: base (name)
+	{}
+}
+
+abstract class LlvmIrVariable : LlvmIrVariableReference, IEquatable<LlvmIrVariable>
+{
 	public Type Type                               { get; protected set; }
 	public LlvmIrVariableWriteOptions WriteOptions { get; set; } = LlvmIrVariableWriteOptions.ArrayWriteIndexComments;
 
@@ -47,21 +80,6 @@ abstract class LlvmIrVariable : IEquatable<LlvmIrVariable>
 	/// will ignore name when checking for equality.
 	protected bool NameMatters { get; set; } = true;
 
-	/// <summary>
-	/// Returns a string which constitutes a reference to a local (using the <c>%</c> prefix character) or a global
-	/// (using the <c>@</c> prefix character) variable, ready for use in the generated code wherever variables are
-	/// referenced.
-	/// </summary>
-	public virtual string Reference {
-		get {
-			if (String.IsNullOrEmpty (Name)) {
-				throw new InvalidOperationException ("Variable doesn't have a name, it cannot be referenced");
-			}
-
-			return $"{NamePrefix}{Name}";
-		}
-	}
-
 	/// <summary>
 	/// <para>
 	/// Certain data must be calculated when the target architecture is known, because it may depend on certain aspects of
@@ -103,6 +121,7 @@ public virtual string Reference {
 	/// is treated as an opaque pointer type.
 	/// </summary>
 	protected LlvmIrVariable (Type type, string? name = null)
+		: base (name)
 	{
 		Type = type;
 		Name = name;
@@ -253,7 +272,7 @@ class LlvmIrGlobalVariable : LlvmIrVariable
 	public LlvmIrStreamedArrayDataProvider? ArrayDataProvider { get; set; }
 
 	/// <summary>
-	/// Constructs a local variable. <paramref name="type"/> is translated to one of the LLVM IR first class types (see
+	/// Constructs a global variable. <paramref name="type"/> is translated to one of the LLVM IR first class types (see
 	/// https://llvm.org/docs/LangRef.html#t-firstclass) only if it's an integral or floating point type.  In all other cases it
 	/// is treated as an opaque pointer type.  <paramref name="name"/> is required because global variables must be named.
 	/// </summary>

src/Xamarin.Android.Build.Tasks/Utilities/PreservePinvokesNativeAssemblyGenerator.cs

@@ -58,6 +58,15 @@ public void Sort ()
 		}
 	}
 
+	sealed class ConstructionState
+	{
+		public LlvmIrFunction Func;
+		public LlvmIrFunctionLabelItem ReturnLabel;
+		public LlvmIrFunctionParameter EntryPointHashParam;
+		public LlvmIrInstructions.Phi Phi;
+		public bool Is64Bit;
+	}
+
 	// Maps a component name after ridding it of the `lib` prefix and the extension to a "canonical"
 	// name of a library, as used in `[DllImport]` attributes.
 	readonly Dictionary<string, string> libraryNameMap = new (StringComparer.Ordinal) {
@@ -181,36 +190,94 @@ void AddFindPinvoke<T> (LlvmIrModule module, List<Component> components, bool is
 			}
 		);
 
-		// TODO: attributes
 		var func = new LlvmIrFunction (sig, MakeFindPinvokeAttributeSet (module)) {
 			CallingConvention = LlvmIrCallingConvention.Fastcc,
 			Linkage = LlvmIrLinkage.Internal,
 		};
-		module.Add (func);
-		func.Body.Add (new LlvmIrFunctionLabelItem ("entry"));
+		LlvmIrLocalVariable retval = func.CreateLocalVariable (typeof(IntPtr), "retval");
+		var state = new ConstructionState {
+			Func = func,
+			ReturnLabel = new LlvmIrFunctionLabelItem ("return"),
+			EntryPointHashParam = parameters[1],
+			Phi = new LlvmIrInstructions.Phi (retval),
+			Is64Bit = is64Bit,
+		};
+		module.Add (state.Func);
+		state.Func.Body.Add (new LlvmIrFunctionLabelItem ("entry"));
 
 		var libraryNameSwitchEpilog = new LlvmIrFunctionLabelItem ("libNameSW.epilog");
 		var componentSwitch = new LlvmIrInstructions.Switch<T> (parameters[0], libraryNameSwitchEpilog, "sw.libname");
-		func.Body.Add (componentSwitch);
+
+		state.Func.Body.Add (componentSwitch);
+		state.Phi.AddNode (libraryNameSwitchEpilog, null);
 
 		components.Sort ((Component a, Component b) => a.NameHash.CompareTo (b.NameHash));
 		Log.LogDebugMessage ("  Components to be preserved:");
+		uint componentID = 1;
+
 		foreach (Component component in components) {
-			component.Sort ();
 			Log.LogDebugMessage ($"    {component.Name} (hash: 0x{component.NameHash:x}; {component.PInvokes.Count} p/invoke(s))");
 
-			LlvmIrFunctionLabelItem componentLabel;
-			if (is64Bit) {
-				componentLabel = componentSwitch.Add ((T)(object)component.NameHash);
+			string comment = $" {component.Name}";
+			LlvmIrFunctionLabelItem componentLabel = AddSwitchItem<T> (componentSwitch, component.NameHash, is64Bit, comment, null);
+
+			func.Body.Add (componentLabel, comment);
+			AddPInvokeSwitch<T> (state, componentLabel, component, componentID++);
+		}
+
+		func.Body.Add (libraryNameSwitchEpilog);
+
+		var setKnownLib = new LlvmIrInstructions.Store (false, parameters[2]);
+		func.Body.Add (setKnownLib);
+		AddReturnBranch (func, state.ReturnLabel);
+
+		func.Body.Add (state.ReturnLabel);
+		func.Body.Add (state.Phi);
+		func.Body.Add (new LlvmIrInstructions.Ret (typeof (IntPtr), retval));
+	}
+
+	void AddPInvokeSwitch<T> (ConstructionState state, LlvmIrFunctionLabelItem componentLabel, Component component, uint id) where T: struct
+	{
+		var pinvokeSwitchEpilog = new LlvmIrFunctionLabelItem ($"pinvokeSW.epilog.{id}");
+		state.Phi.AddNode (pinvokeSwitchEpilog, null);
+
+		var pinvokeSwitch = new LlvmIrInstructions.Switch<T> (state.EntryPointHashParam, pinvokeSwitchEpilog, $"sw.pinvoke.{id}");
+		state.Func.Body.Add (pinvokeSwitch);
+
+		component.Sort ();
+		bool first = true;
+		foreach (PInvoke pi in component.PInvokes) {
+			string pinvokeName = pi.NativeFunction.Signature.Name;
+			string comment = $" {pinvokeName}";
+			LlvmIrFunctionLabelItem pinvokeLabel = AddSwitchItem<T> (pinvokeSwitch, pi.Hash, state.Is64Bit, comment, first ? state.ReturnLabel : null);
+
+			// First item of every component switch block "reuses" the block's label
+			if (first) {
+				first = false;
 			} else {
-				componentLabel = componentSwitch.Add ((T)(object)(uint)component.NameHash);
+				state.Func.Body.Add (pinvokeLabel, comment);
+				AddReturnBranch (state.Func, state.ReturnLabel);
 			}
 
-			func.Body.Add (componentLabel);
-			// TODO: output component `switch` here
+			state.Phi.AddNode (pinvokeLabel == state.ReturnLabel ? componentLabel : pinvokeLabel, new LlvmIrGlobalVariableReference (pinvokeName));
 		}
 
-		func.Body.Add (libraryNameSwitchEpilog);
+		state.Func.Body.Add (pinvokeSwitchEpilog);
+		AddReturnBranch (state.Func, state.ReturnLabel);
+	}
+
+	void AddReturnBranch (LlvmIrFunction func, LlvmIrFunctionLabelItem returnLabel)
+	{
+		var branch = new LlvmIrInstructions.Br (returnLabel);
+		func.Body.Add (branch);
+	}
+
+	LlvmIrFunctionLabelItem AddSwitchItem<T> (LlvmIrInstructions.Switch<T> sw, ulong hash, bool is64Bit, string? comment, LlvmIrFunctionLabelItem? label) where T: struct
+	{
+		if (is64Bit) {
+			return sw.Add ((T)(object)hash, dest: label, comment: comment);
+		}
+		return sw.Add ((T)(object)(uint)hash, dest: label, comment: comment);
 	}
 
 	LlvmIrFunctionAttributeSet MakeFindPinvokeAttributeSet (LlvmIrModule module)