Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more haddocks for AST Instructions #149

Merged
merged 2 commits into from
Dec 9, 2024
Merged

Add more haddocks for AST Instructions #149

merged 2 commits into from
Dec 9, 2024

Conversation

kquick
Copy link
Member

@kquick kquick commented Dec 8, 2024

No description provided.

@kquick kquick requested a review from RyanGlScott December 8, 2024 05:12
Comment on lines 1201 to 1202
the third is the mask,
the third is the insert position.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the sentence is worded confusingly, as it describes "the third" twice in a row, but with different descriptions. Perhaps just shorten this to "the third argument is the mask".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yes, the second line is leftover garbage.

@@ -1196,7 +1196,17 @@ data Instr' lab


| ShuffleVector (Typed (Value' lab)) (Value' lab) (Typed (Value' lab))

{- ^ * Constructs a fixed permutation of two input vectors: the first
argument is the input vector V1, the second input vector V2, and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goes through the effort of naming the input vectors V1 and V2, but it then never uses the names V1 or V2 in the rest of the Haddocks. Perhaps shorten this to "the first and second arguments are the input vectors, and the third argument is the mask".

Comment on lines 1224 to 1229
* Arguments:
1. The FunctionType
2. The Function target
3. arguments
4. onSuccess ret target label
5. onException unwind target label
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some rather unusual capitalization going on here, between:

  • "FunctionType" (is that supposed to be code?)
  • "The Function target" (why is Function randomly capitalized?)
  • "onSuccess"/"onException" (why do these randomly use lowerCamelCase?)

I thought that perhaps these might have been taken from the LLVM Language Reference Manual entry for invoke, but I don't see anything of the sort there, so I'm not sure where these conventions came from.

Comment on lines 1240 to 1242
-- ^ Accesses arguments passed through "varargs" areas of a function call.
-- The argument is a va_list*; this instruction returns the value of the
-- specified type located at the target and increments the pointer.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting nit picks:

Suggested change
-- ^ Accesses arguments passed through "varargs" areas of a function call.
-- The argument is a va_list*; this instruction returns the value of the
-- specified type located at the target and increments the pointer.
-- ^ Accesses arguments passed through \"varargs\" areas of a function call.
-- The argument is a @va_list*@; this instruction returns the value of the
-- specified type located at the target and increments the pointer.

Comment on lines 1251 to 1253
(key values are unique). If the value is not found in the table,
the second argument is the default destination if the target is
not in the table.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of this sentence is redundant. Perhaps:

Suggested change
(key values are unique). If the value is not found in the table,
the second argument is the default destination if the target is
not in the table.
(key values are unique). The second argument is the default
destination if the target is not in the table.

Comment on lines 1257 to 1268
{- ^ Target of an exception (from the Invoke instruction).
* Arguments:
1. The result type (the values set by the personality function
on re-entry to the function).
2. the second argument may be the personality function, which defines
values on re-entry. This is used in older LLVM versions and is
not supplied for recent LLVM versions.
3. True if this block is a "cleanup"
4. The list of clauses to handle the exception; The clauses are
used to match the exception thrown.
* If no clause matches and cleanup not set, continue unwinding up
the stack (see Resume).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting nit picks:

Suggested change
{- ^ Target of an exception (from the Invoke instruction).
* Arguments:
1. The result type (the values set by the personality function
on re-entry to the function).
2. the second argument may be the personality function, which defines
values on re-entry. This is used in older LLVM versions and is
not supplied for recent LLVM versions.
3. True if this block is a "cleanup"
4. The list of clauses to handle the exception; The clauses are
used to match the exception thrown.
* If no clause matches and cleanup not set, continue unwinding up
the stack (see Resume).
{- ^ Target of an exception (from the 'Invoke' instruction).
* Arguments:
1. The result type (the values set by the personality function
on re-entry to the function).
2. the second argument may be the personality function, which defines
values on re-entry. This is used in older LLVM versions and is
not supplied for recent LLVM versions.
3. True if this block is a "cleanup"
4. The list of clauses to handle the exception; The clauses are
used to match the exception thrown.
* If no clause matches and cleanup not set, continue unwinding up
the stack (see 'Resume').


| Resume (Typed (Value' lab))
{- ^ Resumes propagation of an in-flight exception whose unwinding was
interrupted by a LandingPad instruction.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting nit pick:

Suggested change
interrupted by a LandingPad instruction.
interrupted by a 'LandingPad' instruction.

Comment on lines 1279 to 1282
{- ^ * Used to stop propagation of @undef@ and @poison@ values.
* If the argument is undef or poison, returns an arbitrary (but fixed)
value of that type instead, otherwise a no-op and returns its
argument.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency:

Suggested change
{- ^ * Used to stop propagation of @undef@ and @poison@ values.
* If the argument is undef or poison, returns an arbitrary (but fixed)
value of that type instead, otherwise a no-op and returns its
argument.
{- ^ * Used to stop propagation of @undef@ and @poison@ values.
* If the argument is @undef@ or @poison@, returns an arbitrary (but fixed)
value of that type instead, otherwise a no-op and returns its
argument.

@kquick
Copy link
Member Author

kquick commented Dec 9, 2024

Sorry about the bad syntax: I was making these changes as a side-effect of some other work and decided it would be better to commit them than discard them, but I did the former without proofreading and converting in-progress to final product. Hopefully these look better now.

@kquick kquick requested a review from RyanGlScott December 9, 2024 21:54
@kquick kquick merged commit f33aabf into master Dec 9, 2024
1 check passed
@kquick kquick deleted the dgb_1733634572-0 branch December 9, 2024 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants